feat(federation): notify user name changes#39750
Merged
Conversation
Contributor
|
Looks like this PR is ready to merge! 🎉 |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ 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)
🧰 Additional context used📓 Path-based instructions (2)**/*.{ts,tsx,js}📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Files:
**/*.spec.ts📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Files:
🧠 Learnings (21)📓 Common learnings📚 Learning: 2026-03-16T11:57:17.987ZApplied to files:
📚 Learning: 2025-11-24T17:08:17.065ZApplied to files:
📚 Learning: 2025-09-19T15:15:04.642ZApplied to files:
📚 Learning: 2026-01-17T01:51:47.764ZApplied to files:
📚 Learning: 2025-10-28T16:53:42.761ZApplied to files:
📚 Learning: 2025-11-24T17:08:17.065ZApplied to files:
📚 Learning: 2025-11-24T17:08:17.065ZApplied to files:
📚 Learning: 2025-11-05T21:04:35.787ZApplied to files:
📚 Learning: 2025-11-24T17:08:17.065ZApplied to files:
📚 Learning: 2025-12-09T20:01:00.324ZApplied to files:
📚 Learning: 2025-11-04T16:49:19.107ZApplied to files:
📚 Learning: 2025-09-25T09:59:26.461ZApplied to files:
📚 Learning: 2025-09-25T09:59:26.461ZApplied to files:
📚 Learning: 2026-03-09T18:39:21.178ZApplied to files:
📚 Learning: 2025-09-16T13:33:49.237ZApplied to files:
📚 Learning: 2026-03-11T22:04:20.529ZApplied to files:
📚 Learning: 2026-02-24T19:22:48.358ZApplied to files:
📚 Learning: 2026-02-26T19:25:44.063ZApplied to files:
📚 Learning: 2026-02-26T19:25:44.063ZApplied to files:
📚 Learning: 2026-03-06T18:10:15.268ZApplied to files:
🔇 Additional comments (3)
WalkthroughDetects Rocket.Chat user name changes and propagates them to federated Matrix rooms; accepts incoming Matrix displayname changes to update local users. Adds service method, model queries, hooks, debounced event handling, dependency bumps, and end-to-end tests. Changes
Sequence Diagram(s)sequenceDiagram
participant RC as Rocket.Chat
participant Hook as afterSaveUser Hook
participant FM as FederationMatrix
participant DB as Database
participant SDK as Federation SDK
participant Matrix as Matrix Server
RC->>Hook: user update persisted
Hook->>Hook: validate args & native-federated
Hook->>FM: updateUserName(user)
FM->>DB: findJoinedByUserId(userId)
DB-->>FM: subscriptions[]
FM->>DB: findFederatedByIds(roomIds)
DB-->>FM: federated rooms[]
loop per federated room
FM->>SDK: updateRoomMembership (membership: join, content.displayname)
SDK->>Matrix: update member displayname
Matrix-->>SDK: response
SDK-->>FM: result
end
sequenceDiagram
participant Matrix as Matrix Server
participant SDK as Federation SDK
participant Handler as member.handleJoin
participant Debounce as Debounced Update
participant RCDB as Rocket.Chat DB
Matrix->>SDK: join event (with displayname)
SDK->>Handler: deliver event (content)
Handler->>Handler: compare displayname vs stored
Handler->>Debounce: schedule updateUserNameDebounced(userId, name)
Debounce-->>Debounce: wait 2s
Debounce->>RCDB: Users.setName(userId, displayname)
RCDB-->>Debounce: success
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
🦋 Changeset detectedLatest commit: d6d9603 The changes in this PR will be included in the next version bump. This PR includes changesets to release 41 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
sampaiodiego
force-pushed
the
federation-update-name
branch
from
13ff35b to
8d31d26
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #39750 +/- ##
===========================================
+ Coverage 70.48% 70.51% +0.03%
===========================================
Files 3247 3245 -2
Lines 115374 115370 -4
Branches 21020 21002 -18
===========================================
+ Hits 81323 81357 +34
+ Misses 31987 31949 -38
Partials 2064 2064
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
sampaiodiego
force-pushed
the
federation-update-name
branch
from
750745f to
e753d2a
Compare
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
4 issues found across 17 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="ee/packages/federation-matrix/tests/end-to-end/room.spec.ts">
<violation number="1" location="ee/packages/federation-matrix/tests/end-to-end/room.spec.ts:1917">
P2: This critical sync scenario is marked with `it.failing`, which allows the suite to pass even when the new-room display-name propagation remains broken.</violation>
</file>
<file name="apps/meteor/ee/server/hooks/federation/index.ts">
<violation number="1" location="apps/meteor/ee/server/hooks/federation/index.ts:325">
P1: Handle federation update errors in `afterSaveUser` so transient federation failures do not fail user profile updates.</violation>
</file>
<file name="ee/packages/federation-matrix/src/events/member.ts">
<violation number="1" location="ee/packages/federation-matrix/src/events/member.ts:199">
P1: The new debounce is global across all users, so name updates from different users within the debounce window can cancel each other and drop valid updates.
(Based on your team's feedback about concurrency implications in async flows.) [FEEDBACK_USED]</violation>
</file>
<file name="ee/packages/federation-matrix/src/FederationMatrix.ts">
<violation number="1" location="ee/packages/federation-matrix/src/FederationMatrix.ts:895">
P1: Use the existing federated Matrix user ID when present; rebuilding it from the updated username can send updates for the wrong Matrix user after a rename.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| // when a user changes their username, we need to send a new event for every room the user is a member | ||
| async updateUserName(user: IUser): Promise<void> { | ||
| // const oldMatrixUserId = user.federation.mui; | ||
| const matrixUserId = userIdSchema.parse(`@${user.username}:${this.serverName}`); |
Contributor
There was a problem hiding this comment.
P1: Use the existing federated Matrix user ID when present; rebuilding it from the updated username can send updates for the wrong Matrix user after a rename.
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 895:
<comment>Use the existing federated Matrix user ID when present; rebuilding it from the updated username can send updates for the wrong Matrix user after a rename.</comment>
<file context>
@@ -888,4 +888,36 @@ export class FederationMatrix extends ServiceClass implements IFederationMatrixS
+ // when a user changes their username, we need to send a new event for every room the user is a member
+ async updateUserName(user: IUser): Promise<void> {
+ // const oldMatrixUserId = user.federation.mui;
+ const matrixUserId = userIdSchema.parse(`@${user.username}:${this.serverName}`);
+
+ const subs = await Subscriptions.findJoinedByUserId<Pick<ISubscription, 'rid'>>(user._id, { projection: { rid: 1 } }).toArray();
</file context>
ggazzo
reviewed
Mar 20, 2026
ggazzo
reviewed
Mar 20, 2026
coderabbitai
bot
added
type: feature
Contributor
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="ee/packages/federation-matrix/src/events/member.ts">
<violation number="1" location="ee/packages/federation-matrix/src/events/member.ts:205">
P2: The new `Map` of debounced username updaters never evicts entries, causing unbounded in-memory growth as more federated users are processed.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![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: 3
🧹 Nitpick comments (1)
ee/packages/federation-matrix/src/FederationMatrix.ts (1)
894-904: Remove the commented-out debug leftovers.The
oldMatrixUserIdandconsole.loglines are dead comments now and will drift quickly in service code.As per coding guidelines, `**/*.{ts,tsx,js}`: Avoid code comments in the implementation.♻️ Suggested cleanup
- // const oldMatrixUserId = user.federation.mui; const matrixUserId = userIdSchema.parse(`@${user.username}:${this.serverName}`); @@ - // console.log('roomsUserIsMemberOf ->', roomsUserIsMemberOf); - await Promise.all(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ee/packages/federation-matrix/src/FederationMatrix.ts` around lines 894 - 904, Remove the leftover commented debug lines in FederationMatrix.ts: delete the commented declaration "// const oldMatrixUserId = user.federation.mui;" and the "// console.log('roomsUserIsMemberOf ->', roomsUserIsMemberOf);" so the block around matrixUserId (created via userIdSchema.parse), the Subscriptions.findJoinedByUserId call, and the Rooms.findFederatedByIds call contains no dead commented debug code; keep the active variables and queries (matrixUserId, subs, rooms) intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/meteor/ee/server/hooks/federation/index.ts`:
- Around line 324-325: The profile-save path currently awaits
FederationMatrix.updateUserName(userUpdated), causing the request to block on
remote Matrix fan-out; change this to fire-and-forget so the local profile save
is fast and resilient: invoke FederationMatrix.updateUserName(userUpdated)
without awaiting (e.g. call it and attach a .catch handler or wrap in a detached
async task) and ensure errors are only logged locally (use processLogger or
equivalent in the catch). Update the call site where 'name' in userUpdated is
checked to use this non-blocking dispatch pattern.
In `@ee/packages/federation-matrix/src/events/member.ts`:
- Around line 238-243: The current logic only updates a federated user's display
name when handling already-joined users, so the seeded placeholder name from
getOrCreateFederatedUser() remains after an INVITED->joined transition; add the
same displayname handling to the branch that processes the initial join (the
branch handling newly-accepted joins) so that when content.displayname exists
you call updateUserNameDebounced(joiningUser._id, content.displayname || '')
(debounced) for both newly-joined and already-joined flows; reference
getOrCreateFederatedUser(), updateUserNameDebounced, joiningUser,
content.displayname and subscription.status to locate and apply the change.
In `@ee/packages/federation-matrix/tests/end-to-end/room.spec.ts`:
- Line 1918: The test was hidden by using it.failing; change the test
declaration from it.failing('should show the user current display name on the
Synapse side', ...) back to a normal it(...) so failures surface, and fix the
room-creation path to propagate the creator's current display name into the
initial member state for federated rooms: locate the federated room creation
flow (functions like createFederatedRoom / Federation.createRoom /
RoomService.createRoom / onCreateRoom handler) and ensure when emitting the
initial m.room.member event for the creator you set/displayname to the creator's
current display name rather than leaving it blank or stale so Synapse will see
the correct name.
---
Nitpick comments:
In `@ee/packages/federation-matrix/src/FederationMatrix.ts`:
- Around line 894-904: Remove the leftover commented debug lines in
FederationMatrix.ts: delete the commented declaration "// const oldMatrixUserId
= user.federation.mui;" and the "// console.log('roomsUserIsMemberOf ->',
roomsUserIsMemberOf);" so the block around matrixUserId (created via
userIdSchema.parse), the Subscriptions.findJoinedByUserId call, and the
Rooms.findFederatedByIds call contains no dead commented debug code; keep the
active variables and queries (matrixUserId, subs, rooms) intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5671daf5-8df6-4ff7-b9d0-c9160930568d
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (16)
.changeset/five-chicken-invite.mdapps/meteor/ee/server/hooks/federation/index.tsapps/meteor/package.jsonapps/meteor/server/methods/saveUserProfile.tsapps/meteor/server/services/room/service.tsee/packages/federation-matrix/package.jsonee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/events/member.tsee/packages/federation-matrix/tests/end-to-end/room.spec.tspackages/core-services/package.jsonpackages/core-services/src/types/IFederationMatrixService.tspackages/model-typings/src/models/IRoomsModel.tspackages/model-typings/src/models/ISubscriptionsModel.tspackages/models/src/models/Rooms.tspackages/models/src/models/Subscriptions.tspackages/server-fetch/package.json
📜 Review details
⏰ 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). (4)
- GitHub Check: 📦 Build Packages
- GitHub Check: cubic · AI code reviewer
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/server/services/room/service.tspackages/core-services/src/types/IFederationMatrixService.tsapps/meteor/server/methods/saveUserProfile.tspackages/model-typings/src/models/IRoomsModel.tsapps/meteor/ee/server/hooks/federation/index.tspackages/model-typings/src/models/ISubscriptionsModel.tsee/packages/federation-matrix/src/events/member.tspackages/models/src/models/Rooms.tsee/packages/federation-matrix/tests/end-to-end/room.spec.tsee/packages/federation-matrix/src/FederationMatrix.tspackages/models/src/models/Subscriptions.ts
**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.spec.ts: Use descriptive test names that clearly communicate expected behavior in Playwright tests
Use.spec.tsextension for test files (e.g.,login.spec.ts)
Files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
🧠 Learnings (38)
📓 Common learnings
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: smirk-dev
Repo: RocketChat/Rocket.Chat PR: 39625
File: apps/meteor/app/api/server/v1/push.ts:85-97
Timestamp: 2026-03-14T14:58:58.834Z
Learning: In RocketChat/Rocket.Chat, the `push.token` POST/DELETE endpoints in `apps/meteor/app/api/server/v1/push.ts` were already migrated to the chained router API pattern on `develop` prior to PR `#39625`. `cleanTokenResult` (which strips `authToken` and returns `PushTokenResult`) and `isPushTokenPOSTProps`/`isPushTokenDELETEProps` validators already exist on `develop`. PR `#39625` only migrates `push.get` and `push.info` to the chained pattern. Do not flag `cleanTokenResult` or `PushTokenResult` as newly introduced behavior-breaking changes when reviewing this PR.
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.
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38262
File: apps/meteor/client/lib/utils/normalizeMessagePreview/getMessagePreview.ts:24-33
Timestamp: 2026-01-19T18:17:46.433Z
Learning: In the Rocket.Chat repository, usernames (lastMessage.u.username) and display names (lastMessage.u.name) are sanitized/escaped centrally elsewhere in the system, so individual display functions like getMessagePreview do not need to escape these values before interpolating them into strings.
📚 Learning: 2026-02-24T19:05:56.710Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 0
File: :0-0
Timestamp: 2026-02-24T19:05:56.710Z
Learning: Rocket.Chat repo context: When a workspace manifest on develop already pins a dependency version (e.g., packages/web-ui-registration → "rocket.chat/ui-contexts": "27.0.1"), a lockfile change in a feature PR that upgrades only that dependency’s resolution is considered a manifest-driven sync and can be kept, preferably as a small "chore: sync yarn.lock with manifests" commit.
Applied to files:
packages/server-fetch/package.jsonapps/meteor/package.jsonee/packages/federation-matrix/package.jsonpackages/core-services/package.json.changeset/five-chicken-invite.md
📚 Learning: 2026-02-25T20:10:16.987Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38913
File: packages/ddp-client/src/legacy/types/SDKLegacy.ts:34-34
Timestamp: 2026-02-25T20:10:16.987Z
Learning: In the RocketChat/Rocket.Chat monorepo, packages/ddp-client and apps/meteor do not use TypeScript project references. Module augmentations in apps/meteor (e.g., declare module 'rocket.chat/rest-typings') are not visible when compiling packages/ddp-client in isolation, which is why legacy SDK methods that depend on OperationResult types for OpenAPI-migrated endpoints must remain commented out.
Applied to files:
apps/meteor/package.jsonee/packages/federation-matrix/package.jsonpackages/core-services/package.jsonapps/meteor/server/services/room/service.tspackages/model-typings/src/models/IRoomsModel.tsee/packages/federation-matrix/src/FederationMatrix.ts
📚 Learning: 2026-02-24T19:09:09.561Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38974
File: apps/meteor/app/api/server/v1/im.ts:220-221
Timestamp: 2026-02-24T19:09:09.561Z
Learning: In RocketChat/Rocket.Chat OpenAPI migration PRs for apps/meteor/app/api/server/v1 endpoints, maintainers prefer to avoid any logic changes; style-only cleanups (like removing inline comments) may be deferred to follow-ups to keep scope tight.
Applied to files:
apps/meteor/package.json
📚 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:
apps/meteor/package.jsonee/packages/federation-matrix/package.jsonpackages/core-services/package.jsonapps/meteor/ee/server/hooks/federation/index.tsee/packages/federation-matrix/src/events/member.ts.changeset/five-chicken-invite.mdee/packages/federation-matrix/tests/end-to-end/room.spec.tsee/packages/federation-matrix/src/FederationMatrix.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/package.jsonpackages/core-services/package.jsonapps/meteor/server/services/room/service.tspackages/core-services/src/types/IFederationMatrixService.tspackages/model-typings/src/models/IRoomsModel.tsapps/meteor/ee/server/hooks/federation/index.tsee/packages/federation-matrix/src/events/member.tspackages/models/src/models/Rooms.ts.changeset/five-chicken-invite.mdee/packages/federation-matrix/tests/end-to-end/room.spec.tsee/packages/federation-matrix/src/FederationMatrix.ts
📚 Learning: 2026-03-14T14:58:58.834Z
Learnt from: smirk-dev
Repo: RocketChat/Rocket.Chat PR: 39625
File: apps/meteor/app/api/server/v1/push.ts:85-97
Timestamp: 2026-03-14T14:58:58.834Z
Learning: In RocketChat/Rocket.Chat, the `push.token` POST/DELETE endpoints in `apps/meteor/app/api/server/v1/push.ts` were already migrated to the chained router API pattern on `develop` prior to PR `#39625`. `cleanTokenResult` (which strips `authToken` and returns `PushTokenResult`) and `isPushTokenPOSTProps`/`isPushTokenDELETEProps` validators already exist on `develop`. PR `#39625` only migrates `push.get` and `push.info` to the chained pattern. Do not flag `cleanTokenResult` or `PushTokenResult` as newly introduced behavior-breaking changes when reviewing this PR.
Applied to files:
apps/meteor/package.json
📚 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.jsonapps/meteor/ee/server/hooks/federation/index.tsee/packages/federation-matrix/src/events/member.tspackages/models/src/models/Rooms.ts.changeset/five-chicken-invite.mdee/packages/federation-matrix/tests/end-to-end/room.spec.tsee/packages/federation-matrix/src/FederationMatrix.ts
📚 Learning: 2026-02-24T19:05:56.710Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 0
File: :0-0
Timestamp: 2026-02-24T19:05:56.710Z
Learning: In Rocket.Chat PRs, keep feature PRs free of unrelated lockfile-only dependency bumps; prefer reverting lockfile drift or isolating such bumps into a separate "chore" commit/PR, and always use yarn install --immutable with the Yarn version pinned in package.json via Corepack.
Applied to files:
apps/meteor/package.jsonee/packages/federation-matrix/package.jsonpackages/core-services/package.json
📚 Learning: 2026-03-17T16:08:37.572Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 39690
File: packages/ui-voip/package.json:11-11
Timestamp: 2026-03-17T16:08:37.572Z
Learning: In `packages/ui-voip/package.json` (RocketChat/Rocket.Chat), the team deliberately chose to use `rm -rf dist` directly in the `"build"` script instead of `rimraf`, as they decided against introducing the `rimraf` dependency for this package. Do not flag `rm -rf dist` in the ui-voip build script as a cross-platform issue requiring rimraf.
Applied to files:
apps/meteor/package.jsonee/packages/federation-matrix/package.json
📚 Learning: 2025-11-05T21:04:35.787Z
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.787Z
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:
apps/meteor/package.jsonee/packages/federation-matrix/package.jsonapps/meteor/ee/server/hooks/federation/index.ts.changeset/five-chicken-invite.mdee/packages/federation-matrix/tests/end-to-end/room.spec.tsee/packages/federation-matrix/src/FederationMatrix.ts
📚 Learning: 2026-03-11T22:04:20.529Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 39545
File: apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts:59-61
Timestamp: 2026-03-11T22:04:20.529Z
Learning: In `apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts`, the `msg.u._id === uid` early-return in the `streamNewMessage` handler is intentional: the "New messages" indicator is designed to notify about messages from other users only. Self-sent messages — including those sent from a different session/device — are always skipped, by design. Do not flag this as a multi-session regression.
Applied to files:
apps/meteor/server/services/room/service.tsapps/meteor/ee/server/hooks/federation/index.tsee/packages/federation-matrix/src/events/member.tsee/packages/federation-matrix/tests/end-to-end/room.spec.tsee/packages/federation-matrix/src/FederationMatrix.ts
📚 Learning: 2026-03-12T10:26:26.697Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39340
File: apps/meteor/app/api/server/v1/im.ts:1349-1398
Timestamp: 2026-03-12T10:26:26.697Z
Learning: In `apps/meteor/app/api/server/v1/im.ts` (PR `#39340`), the `DmEndpoints` type intentionally includes temporary stub entries for `/v1/im.kick`, `/v1/dm.kick`, `/v1/im.leave`, and `/v1/dm.leave` (using `DmKickProps` and `DmLeaveProps`) even though no route handlers exist for them yet. These stubs were added to preserve type compatibility after removing the original `DmLeaveProps` and related files. They are planned for cleanup in a follow-up PR. Do not flag these as missing implementations when reviewing this file until the follow-up is merged.
Applied to files:
apps/meteor/server/services/room/service.ts
📚 Learning: 2026-03-09T18:39:21.178Z
Learnt from: Harxhit
Repo: RocketChat/Rocket.Chat PR: 39476
File: apps/meteor/server/methods/addAllUserToRoom.ts:0-0
Timestamp: 2026-03-09T18:39:21.178Z
Learning: In apps/meteor/server/methods/addAllUserToRoom.ts, the implementation uses a single cursor pass (Users.find(userFilter).batchSize(100)) that collects both the full user objects (collectedUsers: IUser[]) and their usernames (usernames: string[]) in one iteration. `beforeAddUserToRoom` is then called once with the full usernames batch (preserving batch-validation semantics), and the subsequent subscription/message processing loop iterates over the same stable `collectedUsers` array — no second DB query is made. This avoids any race condition between validation and processing while preserving the original batch-validation behavior.
Applied to files:
apps/meteor/server/services/room/service.tspackages/core-services/src/types/IFederationMatrixService.tspackages/model-typings/src/models/IRoomsModel.tsapps/meteor/ee/server/hooks/federation/index.tspackages/model-typings/src/models/ISubscriptionsModel.tsee/packages/federation-matrix/src/events/member.tspackages/models/src/models/Rooms.tsee/packages/federation-matrix/src/FederationMatrix.tspackages/models/src/models/Subscriptions.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:
apps/meteor/server/services/room/service.tspackages/model-typings/src/models/IRoomsModel.tspackages/model-typings/src/models/ISubscriptionsModel.tspackages/models/src/models/Rooms.tsee/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:
apps/meteor/server/services/room/service.tspackages/core-services/src/types/IFederationMatrixService.tspackages/model-typings/src/models/IRoomsModel.tspackages/model-typings/src/models/ISubscriptionsModel.tsee/packages/federation-matrix/src/events/member.tspackages/models/src/models/Rooms.tsee/packages/federation-matrix/src/FederationMatrix.ts
📚 Learning: 2026-03-11T18:15:53.272Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39425
File: apps/meteor/app/api/server/v1/rooms.ts:294-296
Timestamp: 2026-03-11T18:15:53.272Z
Learning: In Rocket.Chat's `rooms.mediaConfirm/:rid/:fileId` endpoint (apps/meteor/app/api/server/v1/rooms.ts), updating `file.name` from `bodyParams.fileName` without updating `file.path` is intentionally safe. The file path is keyed by `_id` (not by filename), so the stored path remains valid regardless of a rename. `file.name` only affects the display name in the message attachment; do not flag this as a path-divergence issue.
Applied to files:
apps/meteor/server/services/room/service.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: UserBridge.doGetUserRoomIds in packages/apps-engine/src/server/bridges/UserBridge.ts has a bug where it implicitly returns undefined when the app lacks read permission (missing return statement in the else case of the permission check).
Applied to files:
apps/meteor/server/services/room/service.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.
Applied to files:
apps/meteor/server/services/room/service.tspackages/core-services/src/types/IFederationMatrixService.tsapps/meteor/server/methods/saveUserProfile.tspackages/model-typings/src/models/IRoomsModel.tsapps/meteor/ee/server/hooks/federation/index.tspackages/model-typings/src/models/ISubscriptionsModel.tsee/packages/federation-matrix/src/events/member.tspackages/models/src/models/Rooms.tsee/packages/federation-matrix/tests/end-to-end/room.spec.tsee/packages/federation-matrix/src/FederationMatrix.tspackages/models/src/models/Subscriptions.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Applied to files:
apps/meteor/server/services/room/service.tspackages/core-services/src/types/IFederationMatrixService.tsapps/meteor/server/methods/saveUserProfile.tspackages/model-typings/src/models/IRoomsModel.tsapps/meteor/ee/server/hooks/federation/index.tspackages/model-typings/src/models/ISubscriptionsModel.tsee/packages/federation-matrix/src/events/member.tspackages/models/src/models/Rooms.tsee/packages/federation-matrix/tests/end-to-end/room.spec.tsee/packages/federation-matrix/src/FederationMatrix.tspackages/models/src/models/Subscriptions.ts
📚 Learning: 2025-12-09T20:01:07.355Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37532
File: ee/packages/federation-matrix/src/FederationMatrix.ts:920-927
Timestamp: 2025-12-09T20:01:07.355Z
Learning: In Rocket.Chat's federation invite handling (ee/packages/federation-matrix/src/FederationMatrix.ts), when a user rejects an invite via federationSDK.rejectInvite(), the subscription cleanup happens automatically through an event-driven flow: Matrix emits a leave event back, which is processed by handleLeave() in ee/packages/federation-matrix/src/events/member.ts, and that function calls Room.performUserRemoval() to clean up the subscription. No explicit cleanup is needed in the reject branch of handleInvite() because the leave event handler takes care of it.
<!-- </add_learning>
Applied to files:
packages/core-services/src/types/IFederationMatrixService.ts
📚 Learning: 2026-03-09T18:39:14.020Z
Learnt from: Harxhit
Repo: RocketChat/Rocket.Chat PR: 39476
File: apps/meteor/server/methods/addAllUserToRoom.ts:0-0
Timestamp: 2026-03-09T18:39:14.020Z
Learning: When implementing batch processing in server methods, favor a single data pass to collect both the items and any derived fields needed for validation. Use the same dataset for both validation and processing to avoid races between validation and execution, and document the approach in code comments. Apply this pattern to similar Meteor Rocket.Chat server methods under apps/meteor/server/methods to prevent race conditions and ensure consistent batch behavior.
Applied to files:
apps/meteor/server/methods/saveUserProfile.ts
📚 Learning: 2025-11-27T17:56:26.050Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37557
File: apps/meteor/client/views/admin/ABAC/AdminABACRooms.tsx:115-116
Timestamp: 2025-11-27T17:56:26.050Z
Learning: In Rocket.Chat, the GET /v1/abac/rooms endpoint (implemented in ee/packages/abac/src/index.ts) only returns rooms where abacAttributes exists and is not an empty array (query: { abacAttributes: { $exists: true, $ne: [] } }). Therefore, in components consuming this endpoint (like AdminABACRooms.tsx), room.abacAttributes is guaranteed to be defined for all returned rooms, and optional chaining before calling array methods like .join() is sufficient without additional null coalescing.
Applied to files:
packages/model-typings/src/models/IRoomsModel.ts
📚 Learning: 2026-03-15T14:31:28.969Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39647
File: apps/meteor/app/api/server/v1/users.ts:710-757
Timestamp: 2026-03-15T14:31:28.969Z
Learning: In RocketChat/Rocket.Chat, the `UserCreateParamsPOST` type in `apps/meteor/app/api/server/v1/users.ts` (migrated from `packages/rest-typings/src/v1/users/UserCreateParamsPOST.ts`) intentionally has `fields: string` (non-optional) and `settings?: IUserSettings` without a corresponding AJV schema entry. This is a pre-existing divergence carried over verbatim from the original rest-typings source (PR `#39647`). Do not flag this type/schema misalignment during the OpenAPI migration review — it is tracked as a separate follow-up fix.
Applied to files:
apps/meteor/ee/server/hooks/federation/index.tsee/packages/federation-matrix/src/events/member.tsee/packages/federation-matrix/src/FederationMatrix.ts
📚 Learning: 2026-01-17T01:51:47.764Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38219
File: packages/core-typings/src/cloud/Announcement.ts:5-6
Timestamp: 2026-01-17T01:51:47.764Z
Learning: In packages/core-typings/src/cloud/Announcement.ts, the AnnouncementSchema.createdBy field intentionally overrides IBannerSchema.createdBy (object with _id and optional username) with a string enum ['cloud', 'system'] to match existing runtime behavior. This is documented as technical debt with a FIXME comment at apps/meteor/app/cloud/server/functions/syncWorkspace/handleCommsSync.ts:53 and should not be flagged as an error until the runtime behavior is corrected.
Applied to files:
apps/meteor/ee/server/hooks/federation/index.tsee/packages/federation-matrix/src/events/member.tsee/packages/federation-matrix/tests/end-to-end/room.spec.tsee/packages/federation-matrix/src/FederationMatrix.ts
📚 Learning: 2025-12-09T20:01:00.324Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37532
File: ee/packages/federation-matrix/src/FederationMatrix.ts:920-927
Timestamp: 2025-12-09T20:01:00.324Z
Learning: When reviewing federation invite handling in Rocket.Chat (specifically under ee/packages/federation-matrix), understand that rejecting an invite via federationSDK.rejectInvite() triggers an event-driven cleanup: a leave event is emitted and handled by handleLeave() in ee/packages/federation-matrix/src/events/member.ts, which calls Room.performUserRemoval() to remove the subscription. Do not add explicit cleanup in the reject branch of handleInvite(); rely on the existing leave-event flow for cleanup. If making changes, ensure this invariant remains and that any related paths still funnel cleanup through the leave event to avoid duplicate or missing removals.
Applied to files:
ee/packages/federation-matrix/src/events/member.tsee/packages/federation-matrix/tests/end-to-end/room.spec.tsee/packages/federation-matrix/src/FederationMatrix.ts
📚 Learning: 2026-03-15T14:31:23.493Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39647
File: apps/meteor/app/api/server/v1/users.ts:891-899
Timestamp: 2026-03-15T14:31:23.493Z
Learning: In RocketChat/Rocket.Chat, `IUser.inactiveReason` in `packages/core-typings/src/IUser.ts` is typed as `'deactivated' | 'pending_approval' | 'idle_too_long'` (optional, no `null`), but the database stores `null` for newly created users. The Typia-generated `$ref: '#/components/schemas/IUser'` schema therefore correctly rejects `null` for `inactiveReason`. This causes the test "should create a new user with default roles" to fail when response validation is active (TEST_MODE). The fix is to add `| null` to `inactiveReason` in core-typings and rebuild Typia schemas in a separate PR. Do not flag this test failure as a bug introduced by the users.create OpenAPI migration (PR `#39647`). Do not suggest inlining a custom schema to work around it, as migration rules require using `$ref` when a Typia schema exists.
Applied to files:
ee/packages/federation-matrix/src/events/member.tsee/packages/federation-matrix/tests/end-to-end/room.spec.ts
📚 Learning: 2026-03-16T21:50:37.589Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39676
File: .changeset/migrate-users-register-openapi.md:3-3
Timestamp: 2026-03-16T21:50:37.589Z
Learning: For changes related to OpenAPI migrations in Rocket.Chat/OpenAPI, when removing endpoint types and validators from rocket.chat/rest-typings (e.g., UserRegisterParamsPOST, /v1/users.register) document this as a minor changeset (not breaking) per RocketChat/Rocket.Chat-Open-API#150 Rule 7. Note that the endpoint type is re-exposed via a module augmentation .d.ts in the consuming package (e.g., packages/web-ui-registration/src/users-register.d.ts). In reviews, ensure the changeset clearly states: this is a non-breaking change, the major version should not be bumped, and the changeset reflects a minor version bump. Do not treat this as a breaking change during OpenAPI migrations.
Applied to files:
.changeset/five-chicken-invite.md
📚 Learning: 2026-03-06T18:09:17.867Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsx:20-23
Timestamp: 2026-03-06T18:09:17.867Z
Learning: In the RocketChat/Rocket.Chat gazzodown package (`packages/gazzodown`), tests are intended to run under the UTC timezone, but as of PR `#39397` this is NOT yet explicitly enforced in `jest.config.ts` or the `package.json` test scripts (which just run `jest` without `TZ=UTC`). To make timezone-sensitive snapshot tests reliable across all environments, `TZ=UTC` should be added to the test scripts in `package.json` or to `jest.config.ts` via `testEnvironmentOptions.timezone`. Without explicit UTC enforcement, snapshot tests involving date-fns formatted output or `toLocaleString()` will fail for contributors in non-UTC timezones.
Applied to files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
📚 Learning: 2026-03-06T18:02:20.381Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/elements/Timestamp/RelativeTime.spec.tsx:63-70
Timestamp: 2026-03-06T18:02:20.381Z
Learning: In RocketChat/Rocket.Chat, tests in the `packages/gazzodown` package (and likely the broader test suite) are always expected to run in the UTC timezone. This makes `toLocaleString()` output deterministic, so snapshot tests that include locale/timezone-sensitive content (such as `title` attributes from `toLocaleString()`) are stable and do not need to be replaced with targeted assertions.
Applied to files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
📚 Learning: 2025-12-10T21:00:54.909Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37091
File: ee/packages/abac/jest.config.ts:4-7
Timestamp: 2025-12-10T21:00:54.909Z
Learning: Rocket.Chat monorepo: Jest testMatch pattern '<rootDir>/src/**/*.spec.(ts|js|mjs)' is valid in this repo and used across multiple packages (e.g., packages/tools, ee/packages/omnichannel-services). Do not flag it as invalid in future reviews.
Applied to files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
📚 Learning: 2026-03-16T11:57:17.987Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 39657
File: apps/meteor/tests/end-to-end/apps/app-resolve-visitor.ts:43-45
Timestamp: 2026-03-16T11:57:17.987Z
Learning: In `apps/meteor/tests/end-to-end/apps/app-resolve-visitor.ts`, `externalIds` lookups are namespaced per `app.id`. Because `cleanupApps()` is called before each test run and a fresh app is installed (giving a new unique `app.id`), fixed `userId` strings like `'nonexistent-id'` in null-path tests are safe and cannot collide with stale visitor records from previous runs. Do not flag these as needing unique/random values.
Applied to files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
📚 Learning: 2026-03-16T11:50:18.066Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 39657
File: apps/meteor/tests/end-to-end/apps/app-resolve-visitor.ts:18-21
Timestamp: 2026-03-16T11:50:18.066Z
Learning: In `apps/meteor/tests/end-to-end/apps/` (and Livechat E2E tests broadly), calling `createAgent()` and `makeAgentAvailable()` immediately after `updateSetting('Livechat_enabled', true)` — without any intermediate sleep — is an established, non-flaky pattern used across 10+ tests in the codebase. Do not flag this sequence as a potential race condition or suggest adding a delay between them.
Applied to files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Maintain test isolation between test cases in Playwright tests
Applied to files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
📚 Learning: 2026-03-02T16:31:41.304Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 39250
File: apps/meteor/tests/end-to-end/api/livechat/07-queue.ts:1084-1094
Timestamp: 2026-03-02T16:31:41.304Z
Learning: In E2E API tests at apps/meteor/tests/end-to-end/api/livechat/, using sleep(1000) after updateSetting() or updateEESetting() calls in test setup hooks is acceptable and intentional to allow omnichannel settings to propagate their side effects.
Applied to files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
📚 Learning: 2026-02-24T19:22:48.358Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/omnichannel/omnichannel-send-pdf-transcript.spec.ts:66-67
Timestamp: 2026-02-24T19:22:48.358Z
Learning: In Playwright end-to-end tests (e.g., under apps/meteor/tests/e2e/...), prefer locating elements by translated text (getByText) and ARIA roles (getByRole) over data-qa attributes. If translation values change, update the corresponding test locators accordingly. Never use data-qa locators. This guideline applies to all Playwright e2e test specs in the repository and helps keep tests robust to UI text changes and accessible semantics.
Applied to files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
📚 Learning: 2026-03-06T18:10:15.268Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/code/CodeBlock.spec.tsx:47-68
Timestamp: 2026-03-06T18:10:15.268Z
Learning: In tests (especially those using testing-library/dom/jsdom) for Rocket.Chat components, the HTML <code> element has an implicit ARIA role of 'code'. Therefore, screen.getByRole('code') or screen.findByRole('code') will locate <code> elements even without a role attribute. Do not flag findByRole('code') as invalid in reviews; prefer using the implicit role instead of adding role="code" unless necessary for accessibility.
Applied to files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
🔇 Additional comments (15)
packages/server-fetch/package.json (1)
23-23: Looks good — runtime dependency addition is reasonable.Adding
abort-controllerindependenciesfits this package’s runtime fetch use case.apps/meteor/server/services/room/service.ts (1)
43-45: DM-only guard is a solid safety check.This fail-fast check correctly enforces method intent and prevents accidental non-DM updates from proceeding.
packages/core-services/package.json (1)
21-21: LGTM!The
@rocket.chat/federation-sdkversion bump to0.5.0is consistent with the federation feature additions in this PR.apps/meteor/package.json (1)
106-106: LGTM!The dependency version is consistent with other packages in this PR.
packages/model-typings/src/models/IRoomsModel.ts (2)
1-10: LGTM!The import additions align with the new
findFederatedByIdsmethod signature.
348-348: LGTM!The generic method signature with
IRoomNativeFederatedas the default type parameter provides good flexibility. Callers can narrow the type (e.g.,Pick<IRoomNativeFederated, '_id' | 'federation' | 'federated'>) with a matching projection when needed.packages/models/src/models/Rooms.ts (2)
7-7: LGTM!Import addition aligns with the new method implementation.
918-925: LGTM!The implementation correctly:
- Queries for rooms matching the provided IDs with
federated: true- Leverages the existing sparse index on
federated(line 97-99)- Supports generic type narrowing for callers that need specific field projections
ee/packages/federation-matrix/package.json (3)
25-25: LGTM!Version is consistent across all three packages (
packages/core-services,apps/meteor, andee/packages/federation-matrix).
32-32: LGTM!Adding
lodash.debounceis appropriate for the per-user name update debouncing logic described in the PR.
44-44: LGTM!Type definitions correctly placed in devDependencies.
.changeset/five-chicken-invite.md (1)
1-6: LGTM!The changeset correctly marks this as a minor version bump for both affected packages, which is appropriate for a new feature addition.
packages/core-services/src/types/IFederationMatrixService.ts (1)
34-34: LGTM!The new interface method follows the existing patterns and provides a clear contract for the implementation in
FederationMatrix.updateUserName.apps/meteor/server/methods/saveUserProfile.ts (2)
19-19: LGTM!Import addition supports the new callback invocation.
182-186: LGTM!The callback placement is appropriate—after the user profile is fully saved but before external notifications. Passing both
updatedUserandoldUserenables the federation hook to detect name changes by comparison.
| }); | ||
|
|
||
| describe('When a new federated room is created on RC', () => { | ||
| it.failing('should show the user current display name on the Synapse side', async () => { |
Contributor
There was a problem hiding this comment.
Please don't mask the creator-name gap behind it.failing.
This scenario is still broken for newly created federated rooms, but it.failing turns that into a green suite. The room-creation path needs to propagate the creator's current display name before this lands.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ee/packages/federation-matrix/tests/end-to-end/room.spec.ts` at line 1918,
The test was hidden by using it.failing; change the test declaration from
it.failing('should show the user current display name on the Synapse side', ...)
back to a normal it(...) so failures surface, and fix the room-creation path to
propagate the creator's current display name into the initial member state for
federated rooms: locate the federated room creation flow (functions like
createFederatedRoom / Federation.createRoom / RoomService.createRoom /
onCreateRoom handler) and ensure when emitting the initial m.room.member event
for the creator you set/displayname to the creator's current display name rather
than leaving it blank or stale so Synapse will see the correct name.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts (1)
1917-1945:⚠️ Potential issue | 🟠 MajorDon't merge this as a masked, order-dependent case.
it.failingkeeps the suite green while the room-creation path is still missing the creator's current display name, and Line 1918 makes this assertion depend on the previous rename test having already run. Please make the precondition explicit in this case and switch it back to a normalit(...)before merge.🧪 Make the case self-contained while re-enabling it
- it.failing('should show the user current display name on the Synapse side', async () => { - // At this point rcUser1's name is adminUpdatedName from the previous test + it('should show the user current display name on the Synapse side', async () => { + await rc1AdminRequestConfig.request + .post(api('users.update')) + .set(rc1AdminRequestConfig.credentials) + .send({ userId: rcUser1.user._id, data: { name: rcUser1.adminUpdatedName } }) + .expect(200);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ee/packages/federation-matrix/tests/end-to-end/room.spec.ts` around lines 1917 - 1945, The test is marked it.failing and implicitly depends on a previous rename test; make it deterministic by changing it.failing(...) to a normal it(...) and explicitly ensure rcUser1 has the expected display name before creating the room: call the appropriate user/profile update helper to set rcUser1's display name to rcUser1.adminUpdatedName (or use the Matrix profile API) prior to invoke createRoom, then proceed with createRoom, hs1AdminApp.acceptInvitationForRoomName, and the retry/assertion that uses hs1AdminApp.getRoomMembers.
🧹 Nitpick comments (1)
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts (1)
1949-1957: Restore the original Synapse display name here.Line 1957 mutates a shared fixture account, but this cleanup hard-codes the Matrix ID as the baseline. If that fixture's default display name ever changes, later runs inherit the wrong state. Capture the original value before the test and restore that exact string in
afterAll.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ee/packages/federation-matrix/tests/end-to-end/room.spec.ts` around lines 1949 - 1957, Capture the Synapse fixture's current display name before mutating it and restore that exact value in afterAll instead of hard-coding the Matrix ID; specifically, in this describe block add a beforeAll that reads and stores the existing display name (e.g., via hs1AdminApp.matrixClient profile/displayName getter) into a variable like originalSynapseDisplayName, use newSynapseDisplayName for the test action, and then call hs1AdminApp.matrixClient.setDisplayName(originalSynapseDisplayName) in afterAll to restore the original state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@ee/packages/federation-matrix/tests/end-to-end/room.spec.ts`:
- Around line 1917-1945: The test is marked it.failing and implicitly depends on
a previous rename test; make it deterministic by changing it.failing(...) to a
normal it(...) and explicitly ensure rcUser1 has the expected display name
before creating the room: call the appropriate user/profile update helper to set
rcUser1's display name to rcUser1.adminUpdatedName (or use the Matrix profile
API) prior to invoke createRoom, then proceed with createRoom,
hs1AdminApp.acceptInvitationForRoomName, and the retry/assertion that uses
hs1AdminApp.getRoomMembers.
---
Nitpick comments:
In `@ee/packages/federation-matrix/tests/end-to-end/room.spec.ts`:
- Around line 1949-1957: Capture the Synapse fixture's current display name
before mutating it and restore that exact value in afterAll instead of
hard-coding the Matrix ID; specifically, in this describe block add a beforeAll
that reads and stores the existing display name (e.g., via
hs1AdminApp.matrixClient profile/displayName getter) into a variable like
originalSynapseDisplayName, use newSynapseDisplayName for the test action, and
then call hs1AdminApp.matrixClient.setDisplayName(originalSynapseDisplayName) in
afterAll to restore the original state.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6a0164b8-004f-4b94-8222-6ce9ea456c0e
📒 Files selected for processing (4)
apps/meteor/ee/server/hooks/federation/index.tsee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/events/member.tsee/packages/federation-matrix/tests/end-to-end/room.spec.ts
✅ Files skipped from review due to trivial changes (1)
- ee/packages/federation-matrix/src/events/member.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- ee/packages/federation-matrix/src/FederationMatrix.ts
- apps/meteor/ee/server/hooks/federation/index.ts
📜 Review details
⏰ 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). (4)
- GitHub Check: 📦 Build Packages
- GitHub Check: cubic · AI code reviewer
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.spec.ts: Use descriptive test names that clearly communicate expected behavior in Playwright tests
Use.spec.tsextension for test files (e.g.,login.spec.ts)
Files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
🧠 Learnings (23)
📓 Common learnings
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.
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38262
File: apps/meteor/client/lib/utils/normalizeMessagePreview/getMessagePreview.ts:24-33
Timestamp: 2026-01-19T18:17:46.433Z
Learning: In the Rocket.Chat repository, usernames (lastMessage.u.username) and display names (lastMessage.u.name) are sanitized/escaped centrally elsewhere in the system, so individual display functions like getMessagePreview do not need to escape these values before interpolating them into strings.
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39425
File: apps/meteor/app/api/server/v1/rooms.ts:294-296
Timestamp: 2026-03-11T18:15:53.272Z
Learning: In Rocket.Chat's `rooms.mediaConfirm/:rid/:fileId` endpoint (apps/meteor/app/api/server/v1/rooms.ts), updating `file.name` from `bodyParams.fileName` without updating `file.path` is intentionally safe. The file path is keyed by `_id` (not by filename), so the stored path remains valid regardless of a rename. `file.name` only affects the display name in the message attachment; do not flag this as a path-divergence issue.
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.
📚 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:
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
📚 Learning: 2026-03-16T11:57:17.987Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 39657
File: apps/meteor/tests/end-to-end/apps/app-resolve-visitor.ts:43-45
Timestamp: 2026-03-16T11:57:17.987Z
Learning: In `apps/meteor/tests/end-to-end/apps/app-resolve-visitor.ts`, `externalIds` lookups are namespaced per `app.id`. Because `cleanupApps()` is called before each test run and a fresh app is installed (giving a new unique `app.id`), fixed `userId` strings like `'nonexistent-id'` in null-path tests are safe and cannot collide with stale visitor records from previous runs. Do not flag these as needing unique/random values.
Applied to files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.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/tests/end-to-end/room.spec.ts
📚 Learning: 2026-03-15T14:31:23.493Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39647
File: apps/meteor/app/api/server/v1/users.ts:891-899
Timestamp: 2026-03-15T14:31:23.493Z
Learning: In RocketChat/Rocket.Chat, `IUser.inactiveReason` in `packages/core-typings/src/IUser.ts` is typed as `'deactivated' | 'pending_approval' | 'idle_too_long'` (optional, no `null`), but the database stores `null` for newly created users. The Typia-generated `$ref: '#/components/schemas/IUser'` schema therefore correctly rejects `null` for `inactiveReason`. This causes the test "should create a new user with default roles" to fail when response validation is active (TEST_MODE). The fix is to add `| null` to `inactiveReason` in core-typings and rebuild Typia schemas in a separate PR. Do not flag this test failure as a bug introduced by the users.create OpenAPI migration (PR `#39647`). Do not suggest inlining a custom schema to work around it, as migration rules require using `$ref` when a Typia schema exists.
Applied to files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
📚 Learning: 2026-03-06T18:09:17.867Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsx:20-23
Timestamp: 2026-03-06T18:09:17.867Z
Learning: In the RocketChat/Rocket.Chat gazzodown package (`packages/gazzodown`), tests are intended to run under the UTC timezone, but as of PR `#39397` this is NOT yet explicitly enforced in `jest.config.ts` or the `package.json` test scripts (which just run `jest` without `TZ=UTC`). To make timezone-sensitive snapshot tests reliable across all environments, `TZ=UTC` should be added to the test scripts in `package.json` or to `jest.config.ts` via `testEnvironmentOptions.timezone`. Without explicit UTC enforcement, snapshot tests involving date-fns formatted output or `toLocaleString()` will fail for contributors in non-UTC timezones.
Applied to files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
📚 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/tests/end-to-end/room.spec.ts
📚 Learning: 2026-03-11T22:04:20.529Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 39545
File: apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts:59-61
Timestamp: 2026-03-11T22:04:20.529Z
Learning: In `apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts`, the `msg.u._id === uid` early-return in the `streamNewMessage` handler is intentional: the "New messages" indicator is designed to notify about messages from other users only. Self-sent messages — including those sent from a different session/device — are always skipped, by design. Do not flag this as a multi-session regression.
Applied to files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
📚 Learning: 2026-03-16T11:50:18.066Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 39657
File: apps/meteor/tests/end-to-end/apps/app-resolve-visitor.ts:18-21
Timestamp: 2026-03-16T11:50:18.066Z
Learning: In `apps/meteor/tests/end-to-end/apps/` (and Livechat E2E tests broadly), calling `createAgent()` and `makeAgentAvailable()` immediately after `updateSetting('Livechat_enabled', true)` — without any intermediate sleep — is an established, non-flaky pattern used across 10+ tests in the codebase. Do not flag this sequence as a potential race condition or suggest adding a delay between them.
Applied to files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
📚 Learning: 2026-01-17T01:51:47.764Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38219
File: packages/core-typings/src/cloud/Announcement.ts:5-6
Timestamp: 2026-01-17T01:51:47.764Z
Learning: In packages/core-typings/src/cloud/Announcement.ts, the AnnouncementSchema.createdBy field intentionally overrides IBannerSchema.createdBy (object with _id and optional username) with a string enum ['cloud', 'system'] to match existing runtime behavior. This is documented as technical debt with a FIXME comment at apps/meteor/app/cloud/server/functions/syncWorkspace/handleCommsSync.ts:53 and should not be flagged as an error until the runtime behavior is corrected.
Applied to files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
📚 Learning: 2026-03-11T18:15:53.272Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39425
File: apps/meteor/app/api/server/v1/rooms.ts:294-296
Timestamp: 2026-03-11T18:15:53.272Z
Learning: In Rocket.Chat's `rooms.mediaConfirm/:rid/:fileId` endpoint (apps/meteor/app/api/server/v1/rooms.ts), updating `file.name` from `bodyParams.fileName` without updating `file.path` is intentionally safe. The file path is keyed by `_id` (not by filename), so the stored path remains valid regardless of a rename. `file.name` only affects the display name in the message attachment; do not flag this as a path-divergence issue.
Applied to files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
📚 Learning: 2026-02-24T19:36:55.089Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/page-objects/fragments/home-content.ts:60-82
Timestamp: 2026-02-24T19:36:55.089Z
Learning: In RocketChat/Rocket.Chat e2e tests (apps/meteor/tests/e2e/page-objects/fragments/home-content.ts), thread message preview listitems do not have aria-roledescription="message", so lastThreadMessagePreview locator cannot be scoped to messageListItems (which filters for aria-roledescription="message"). It should remain scoped to page.getByRole('listitem') or mainMessageList.getByRole('listitem').
Applied to files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
📚 Learning: 2025-11-05T21:04:35.787Z
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.787Z
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/tests/end-to-end/room.spec.ts
📚 Learning: 2026-03-09T18:39:21.178Z
Learnt from: Harxhit
Repo: RocketChat/Rocket.Chat PR: 39476
File: apps/meteor/server/methods/addAllUserToRoom.ts:0-0
Timestamp: 2026-03-09T18:39:21.178Z
Learning: In apps/meteor/server/methods/addAllUserToRoom.ts, the implementation uses a single cursor pass (Users.find(userFilter).batchSize(100)) that collects both the full user objects (collectedUsers: IUser[]) and their usernames (usernames: string[]) in one iteration. `beforeAddUserToRoom` is then called once with the full usernames batch (preserving batch-validation semantics), and the subsequent subscription/message processing loop iterates over the same stable `collectedUsers` array — no second DB query is made. This avoids any race condition between validation and processing while preserving the original batch-validation behavior.
Applied to files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
📚 Learning: 2026-03-02T16:31:41.304Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 39250
File: apps/meteor/tests/end-to-end/api/livechat/07-queue.ts:1084-1094
Timestamp: 2026-03-02T16:31:41.304Z
Learning: In E2E API tests at apps/meteor/tests/end-to-end/api/livechat/, using sleep(1000) after updateSetting() or updateEESetting() calls in test setup hooks is acceptable and intentional to allow omnichannel settings to propagate their side effects.
Applied to files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
📚 Learning: 2026-03-06T18:02:20.381Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/elements/Timestamp/RelativeTime.spec.tsx:63-70
Timestamp: 2026-03-06T18:02:20.381Z
Learning: In RocketChat/Rocket.Chat, tests in the `packages/gazzodown` package (and likely the broader test suite) are always expected to run in the UTC timezone. This makes `toLocaleString()` output deterministic, so snapshot tests that include locale/timezone-sensitive content (such as `title` attributes from `toLocaleString()`) are stable and do not need to be replaced with targeted assertions.
Applied to files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Maintain test isolation between test cases in Playwright tests
Applied to files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
📚 Learning: 2025-12-09T20:01:00.324Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37532
File: ee/packages/federation-matrix/src/FederationMatrix.ts:920-927
Timestamp: 2025-12-09T20:01:00.324Z
Learning: When reviewing federation invite handling in Rocket.Chat (specifically under ee/packages/federation-matrix), understand that rejecting an invite via federationSDK.rejectInvite() triggers an event-driven cleanup: a leave event is emitted and handled by handleLeave() in ee/packages/federation-matrix/src/events/member.ts, which calls Room.performUserRemoval() to remove the subscription. Do not add explicit cleanup in the reject branch of handleInvite(); rely on the existing leave-event flow for cleanup. If making changes, ensure this invariant remains and that any related paths still funnel cleanup through the leave event to avoid duplicate or missing removals.
Applied to files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
📚 Learning: 2026-02-24T19:22:48.358Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/omnichannel/omnichannel-send-pdf-transcript.spec.ts:66-67
Timestamp: 2026-02-24T19:22:48.358Z
Learning: In Playwright end-to-end tests (e.g., under apps/meteor/tests/e2e/...), prefer locating elements by translated text (getByText) and ARIA roles (getByRole) over data-qa attributes. If translation values change, update the corresponding test locators accordingly. Never use data-qa locators. This guideline applies to all Playwright e2e test specs in the repository and helps keep tests robust to UI text changes and accessible semantics.
Applied to files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.
Applied to files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Applied to files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
📚 Learning: 2026-03-06T18:10:15.268Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/code/CodeBlock.spec.tsx:47-68
Timestamp: 2026-03-06T18:10:15.268Z
Learning: In tests (especially those using testing-library/dom/jsdom) for Rocket.Chat components, the HTML <code> element has an implicit ARIA role of 'code'. Therefore, screen.getByRole('code') or screen.findByRole('code') will locate <code> elements even without a role attribute. Do not flag findByRole('code') as invalid in reviews; prefer using the implicit role instead of adding role="code" unless necessary for accessibility.
Applied to files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
🔇 Additional comments (1)
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts (1)
1870-1914: Nice coverage of both local rename paths.Exercising
users.updateOwnBasicInfoandusers.updateseparately is worthwhile here because they hit different server paths but should converge on the same federated member state.
Contributor
There was a problem hiding this comment.
2 issues found across 4 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="ee/packages/federation-matrix/src/events/member.ts">
<violation number="1" location="ee/packages/federation-matrix/src/events/member.ts:209">
P2: Deleting the debounced handler inside the async callback can create parallel debouncers for the same user, causing race-prone/out-of-order name updates under bursty join events.</violation>
</file>
<file name="apps/meteor/ee/server/hooks/federation/index.ts">
<violation number="1" location="apps/meteor/ee/server/hooks/federation/index.ts:325">
P2: Fire-and-forget `updateUserName` should still handle rejections; currently failures can become unhandled promise rejections.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| } | ||
|
|
||
| if ('name' in userUpdated && userUpdated.name !== oldUserData.name) { | ||
| void FederationMatrix.updateUserName(userUpdated); |
Contributor
There was a problem hiding this comment.
P2: Fire-and-forget updateUserName should still handle rejections; currently failures can become unhandled promise rejections.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/ee/server/hooks/federation/index.ts, line 325:
<comment>Fire-and-forget `updateUserName` should still handle rejections; currently failures can become unhandled promise rejections.</comment>
<file context>
@@ -322,6 +322,6 @@ callbacks.add('afterSaveUser', async ({ user: userUpdated, oldUser: oldUserData
if ('name' in userUpdated && userUpdated.name !== oldUserData.name) {
- await FederationMatrix.updateUserName(userUpdated);
+ void FederationMatrix.updateUserName(userUpdated);
}
});
</file context>
Suggested change
| void FederationMatrix.updateUserName(userUpdated); | |
| void FederationMatrix.updateUserName(userUpdated).catch((error) => { | |
| console.error('[updateUserName] Failed to propagate user name change to federation:', error); | |
| }); |
sampaiodiego
force-pushed
the
federation-update-name
branch
from
4dc4191 to
afc2ae2
Compare
ggazzo
previously approved these changes
Mar 20, 2026
ggazzo
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
ggazzo
approved these changes
Mar 20, 2026
coderabbitai
bot
removed
the
type: feature
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes (including videos or screenshots)
Issue(s)
FGA-13
depends on RocketChat/homeserver#381
Steps to test or reproduce
Further comments
Summary by CodeRabbit