test: add end-to-end tests for rejecting an invitation from Synapse #37740
Conversation
|
Looks like this PR is ready to merge! 🎉 |
|
|
Warning Rate limit exceeded@ggazzo has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 21 minutes and 39 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughTightens selection of pending invitations in end-to-end tests by filtering subscriptions by Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts (2)
1644-1647: Consider verifying rejection on both RC and Synapse sides.The test currently only verifies the API response success. For a comprehensive end-to-end test, consider adding:
- Verification that the RC user is not in the room's member list after rejection
- Verification on the Synapse side that the rejection was propagated (similar to how acceptance tests verify membership via
hs1AdminApp.findRoomMember)Example verification to add after the rejection:
it('It should allow RC user to reject the invite', async () => { const rejectResponse = await rejectRoomInvite(rid, rc1AdminRequestConfig); expect(rejectResponse.success).toBe(true); + + // Verify RC user is not in the members list + const members = await getRoomMembers(rid, rc1AdminRequestConfig); + expect(members.members.find((m: IUser) => m.username === federationConfig.rc1.adminUser)).toBeUndefined(); + + // Verify rejection is reflected on Synapse side + const rcAdminInSynapse = await hs1AdminApp.findRoomMember(channelName, federationConfig.rc1.adminMatrixUserId); + expect(rcAdminInSynapse).toBeNull(); });
1649-1655: Add delay before subscription check to avoid race conditions.The test immediately checks for subscription removal after the rejection in the previous test case. Since subscription cleanup may be asynchronous, consider adding a small delay or retry logic to ensure the operation completes.
Apply this pattern (used elsewhere in the codebase):
it.failing('It should remove the subscription after rejection', async () => { + // Allow time for subscription cleanup to complete + await new Promise((resolve) => setTimeout(resolve, 1000)); + const subscriptions = await getSubscriptions(rc1AdminRequestConfig); const invitedSub = subscriptions.update.find((sub) => sub.fname?.includes(channelName)); expect(invitedSub).toBeFalsy(); });Note: The
.failing()modifier indicates this is a known issue. Once the underlying functionality is implemented, remove the.failing()modifier and ensure the test passes reliably.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts(1 hunks)
🧰 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 (5)
📓 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.
📚 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: 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: 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
🧬 Code graph analysis (1)
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts (1)
apps/meteor/tests/data/rooms.helper.ts (2)
getSubscriptions(467-479)rejectRoomInvite(492-508)
⏰ 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). (2)
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (1)
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts (1)
1624-1642: Test setup follows established patterns.The test suite setup correctly creates a Synapse-originated room, invites the RC admin, and waits for the INVITED subscription. The approach is consistent with the existing "Inviting a RC user from Synapse" test suite above (lines 1584-1622), and the unique channel naming ensures proper test isolation for parallel execution.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts (1)
1653-1659: Consider polling for subscription removal with timeout.The failing test checks for subscription removal immediately after the previous rejection test completes. Since subscription removal may be asynchronous, consider adding an explicit polling mechanism with a timeout to wait for the subscription to be removed, similar to how
findRoomMemberuses polling with delays in other tests.Additionally, verify whether the expected behavior is subscription removal or a status change (e.g., from 'INVITED' to 'REJECTED'). If the subscription persists with a different status, the test logic should be adjusted accordingly.
Example polling pattern:
it.failing('It should remove the subscription after rejection', async () => { // Poll for subscription removal with timeout let invitedSub; const maxAttempts = 10; for (let attempt = 0; attempt < maxAttempts; attempt++) { await new Promise((resolve) => setTimeout(resolve, 500)); const subscriptions = await getSubscriptions(rc1AdminRequestConfig); invitedSub = subscriptions.update.find((sub) => sub.fname?.includes(channelName)); if (!invitedSub) break; } expect(invitedSub).toBeFalsy(); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts(2 hunks)
🧰 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 (1)
📚 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
⏰ 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). (2)
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (1)
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts (1)
1601-1603: LGTM! Improved specificity for invitation polling.The addition of
subscription.fname?.includes(channelName)makes the pending invitation search more precise and reduces false positives when multiple invitations exist. The optional chaining safely handles cases wherefnamemight be undefined.
📦 Docker Image Size Report📈 Changes
📊 Historical Trend---
config:
theme: "dark"
xyChart:
width: 900
height: 400
---
xychart
title "Image Size Evolution by Service (Last 30 Days + This PR)"
x-axis ["11/15 22:28", "11/16 01:28", "11/17 23:50", "11/18 22:53", "11/19 23:02", "11/21 16:49", "11/24 17:34", "11/27 22:32", "11/28 19:05", "12/01 23:01", "12/02 21:57", "12/03 21:00", "12/04 18:17", "12/05 21:56", "12/08 20:15", "12/09 22:17", "12/10 02:17", "12/10 02:26 (PR)"]
y-axis "Size (GB)" 0 --> 0.5
line "account-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
line "authorization-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
line "ddp-streamer-service" [0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12]
line "omnichannel-transcript-service" [0.14, 0.14, 0.14, 0.14, 0.14, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13]
line "presence-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
line "queue-worker-service" [0.14, 0.14, 0.14, 0.14, 0.14, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13]
line "rocketchat" [0.36, 0.36, 0.35, 0.35, 0.35, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.35]
line "stream-hub-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
Statistics (last 17 days):
ℹ️ About this reportThis report compares Docker image sizes from this build against the
Updated: Wed, 10 Dec 2025 02:26:08 GMT |
sampaiodiego
force-pushed
the
chore/fed-emit
branch
from
50f76bd to
5555b14
Compare
jessicaschelly
force-pushed
the
test/fed-reject-invite
branch
from
173b670 to
efa17cd
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts (2)
1631-1651: Consider retry/polling around subscription discovery to avoid timing flakes
beforeAllfetches subscriptions only once immediately after the Matrix invite and assertspendingInvitationis defined. If the bridge propagation is slightly delayed, this can intermittently fail even though the invite eventually appears. If you see flakes here, consider wrappinggetSubscriptionsin a small retry/poll loop (similar to howfindRoomMemberuses delays) before asserting and extractingrid. Otherwise, the rejection path and assertion onsuccess === truelook correct.
1653-1659: Clarify whether you require full subscription removal vs just clearing INVITED stateThis test currently asserts that no subscription with
fnamecontainingchannelNameremains insubscriptions.updateafter rejection. If the desired contract is “invites are fully removed,” this is correct. If, instead, the server should retain a subscription row with a different status (e.g.,REJECTEDor similar) but stop treating it as an active invite, you may want to either:
- filter by
status === 'INVITED'here, or- additionally assert on the absence/presence of that subscription in whatever collection represents removed entries.
Please double-check that this expectation matches the intended server behaviour so the test doesn’t over-constrain the implementation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts(2 hunks)
🧰 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 (2)
📚 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: 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
🧬 Code graph analysis (1)
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts (2)
ee/packages/federation-matrix/tests/helper/config.ts (1)
federationConfig(127-127)apps/meteor/tests/data/rooms.helper.ts (2)
getSubscriptions(467-479)rejectRoomInvite(492-508)
🔇 Additional comments (1)
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts (1)
1601-1603: Tightened INVITED subscription filter is consistent and saferNarrowing
pendingInvitationlookup tostatus === 'INVITED'plusfname?.includes(channelName)matches the intended “pending invite for this room” semantics and aligns with the new rejection tests later in the file. Looks good.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## chore/fed-emit #37740 +/- ##
=================================================
Coverage ? 67.81%
=================================================
Files ? 3448
Lines ? 113929
Branches ? 20924
=================================================
Hits ? 77258
Misses ? 34539
Partials ? 2132
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
chore/fed-emit
branch
from
5555b14 to
917d175
Compare
ggazzo
force-pushed
the
chore/fed-emit
branch
from
917d175 to
a9db532
Compare
ggazzo
force-pushed
the
test/fed-reject-invite
branch
from
09f7388 to
18425f1
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts (1)
1-12: Add missing imports for helper functions.The new test code uses
getSubscriptions(lines 1597, 1635, 1652) andgetRoomMembers(line 1612), but these helpers are not imported. This will cause compilation errors.Add the missing imports to the rooms.helper import statement:
import { createRoom, getRoomInfo, getGroupHistory, findRoomMember, addUserToRoom, addUserToRoomSlashCommand, acceptRoomInvite, rejectRoomInvite, + getRoomMembers, } from '../../../../../apps/meteor/tests/data/rooms.helper';And add
getSubscriptionsto the appropriate import (likely from a subscriptions helper or users helper):import { getSubscriptions } from '../../../../../apps/meteor/tests/data/subscriptions.helper';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts(1 hunks)
🧰 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 (5)
📓 Common learnings
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: 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>
📚 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: 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: 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: 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
🧬 Code graph analysis (1)
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts (2)
apps/meteor/tests/data/rooms.helper.ts (3)
rejectRoomInvite(468-484)acceptRoomInvite(439-455)getRoomMembers(259-274)packages/core-typings/src/IUser.ts (1)
IUser(186-256)
⏰ 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). (1)
- GitHub Check: CodeQL-Build
🔇 Additional comments (2)
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts (2)
1582-1658: Good test coverage for Synapse-originated invite flows.The new test suites provide comprehensive coverage for invitation scenarios originating from Synapse, mirroring the existing RC-originated tests. This ensures bidirectional federation invite handling is properly tested.
1651-1657: The subscription cleanup mechanism is properly implemented; no changes needed.The federation event-driven flow correctly handles subscription removal: when a user rejects an invite via
federationSDK.rejectInvite(), Matrix emits a leave event that triggershandleLeave()inmember.ts(line 218), which callsRoom.performUserRemoval()(line 235) to remove the subscription. The test at lines 1651-1657 correctly expects the subscription to be absent after rejection. Noit.failingmarker is needed.Likely an incorrect or invalid review comment.
ggazzo
force-pushed
the
test/fed-reject-invite
branch
from
18425f1 to
682ecc9
Compare
ggazzo
force-pushed
the
test/fed-reject-invite
branch
from
682ecc9 to
b4a2ded
Compare
ggazzo
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
Proposed changes (including videos or screenshots)
Adds end-to-end tests covering the scenario where a Rocket.Chat user rejects a federated room invitation originating from a Synapse homeserver.
What this PR does:
Issue(s)
https://rocketchat.atlassian.net/browse/QA-108
Steps to test or reproduce
Further comments
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.