fix: remove deprecated @ts-ignore in federation-matrix

[mdawoud27]

Proposed changes (including videos or screenshots)

Improved type safety in @rocket.chat/federation-matrix by removing deprecated @ts-ignore comments and fixing a non-null assertion.

Issue(s)

N/A

Steps to test or reproduce

• Run lint to verify warnings are fixed:

   yarn lint

  Expected result: Exit code 0, no warnings (previously had 7 warnings)

• Run tests to ensure no functionality broke

  yarn testunit

  Expected result: All 82 tests pass

Summary by CodeRabbit

• Chores

  • Removed TypeScript ignore directives from validation schemas to improve code quality and enable proper type-checking.

• Tests

  • Enhanced test robustness by improving conditional safety checks in end-to-end tests.

[dionisio-bot]

Looks like this PR is not ready to merge, because of the following issues:

• This PR is missing the 'stat: QA assured' label
• This PR is missing the required milestone or project

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

[changeset-bot]

⚠️ No Changeset found

Latest commit: 5607ec4

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[coderabbitai]

Walkthrough

TypeScript ignore directives are removed from AJV-generated validators in federation matrix API modules. Additionally, a test is made more defensive by replacing a non-null assertion with a conditional guard, preventing potential runtime errors.

Changes

Cohort / File(s)	Summary
TypeScript Directive Removal ee/packages/federation-matrix/src/api/_matrix/profiles.ts, ee/packages/federation-matrix/src/api/_matrix/send-join.ts	Removed // @ts-ignore`` pragmas (3 lines each) before AJV-compiled validators, allowing natural TypeScript type-checking to resume without suppression.
Test Safety Improvement ee/packages/federation-matrix/tests/end-to-end/dms.spec.ts	Replaced non-null assertion with conditional guard on room variable to prevent runtime errors when room is undefined.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 TypeScript whispers, "Let me see!"
As ignore signs fade with glee,
The room's now guarded, safe and sound,
No hidden errors shall abound! ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: removing deprecated @ts-ignore comments from the federation-matrix package to improve type safety.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments

ee/packages/federation-matrix/tests/end-to-end/dms.spec.ts (1)

404-407: Conditional guard silently skips the membership assertion if room is falsy.

Since expect(room).toBeDefined() on line 404 already throws (triggering a retry) when room is falsy, the if (room) guard is redundant. If you want to remove the ! non-null assertion while keeping the test equally strict, move the assertion outside the conditional — TypeScript will narrow the type after the expect throws.

Suggested approach

                     expect(room).toBeDefined();
-                    if (room) {
-                        expect(room.getMyMembership()).toBe('invite');
-                    }
+                    expect(room?.getMyMembership()).toBe('invite');

Using optional chaining here avoids the ! operator while still always evaluating the expect, so a falsy room produces a clear expected undefined to be 'invite' failure instead of silently passing.

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between eb366e7 and 5607ec4.

📒 Files selected for processing (3)

• ee/packages/federation-matrix/src/api/_matrix/profiles.ts
• ee/packages/federation-matrix/src/api/_matrix/send-join.ts
• ee/packages/federation-matrix/tests/end-to-end/dms.spec.ts

💤 Files with no reviewable changes (2)

• ee/packages/federation-matrix/src/api/_matrix/send-join.ts
• ee/packages/federation-matrix/src/api/_matrix/profiles.ts

🧰 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/dms.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.ts extension for test files (e.g., login.spec.ts)

Files:

• ee/packages/federation-matrix/tests/end-to-end/dms.spec.ts

🧠 Learnings (7)

📓 Common learnings

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: 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 : Use `expect` matchers for assertions (`toEqual`, `toContain`, `toBeTruthy`, `toHaveLength`, etc.) instead of `assert` statements in Playwright tests

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

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.

📚 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/dms.spec.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:

• ee/packages/federation-matrix/tests/end-to-end/dms.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/dms.spec.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:

• ee/packages/federation-matrix/tests/end-to-end/dms.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/dms.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/dms.spec.ts

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review 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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[cubic-dev-ai]

No issues found across 3 files