chore: Use published federation package #37046
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
WalkthroughReplaced internal Changes
Sequence Diagram(s)(omitted — changes are dependency/import updates and CI configuration; no runtime control-flow changes to visualize) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (2)📓 Common learnings📚 Learning: 2025-09-19T15:15:04.642ZApplied to files:
⏰ 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 comments (1)
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
ee/packages/federation-matrix/src/api/_matrix/transactions.ts (1)
308-364: Fix broken validators for state_ids/state routes (params vs query mismatch)Both routes validate “params” with a schema that requires event_id, but event_id is a query param. This will 400 the requests.
Apply:
+// Validate path params for routes with :roomId +const RoomIdParamSchema = { + type: 'object', + properties: { + roomId: { + type: 'string', + pattern: '^![A-Za-z0-9_=\\/.+-]+:(.+)$', + description: 'Matrix room ID in format !room:server.com', + }, + }, + required: ['roomId'], + additionalProperties: false, +}; +const isRoomIdParamsProps = ajv.compile(RoomIdParamSchema); + +// Validate query string requiring ?event_id=$... +const StateEventIdQuerySchema = { + type: 'object', + properties: { + event_id: { + type: 'string', + pattern: '^\\$[A-Za-z0-9_=\\/.+-]+(:(.+))?$', + description: 'Matrix event ID in format $event', + }, + }, + required: ['event_id'], + additionalProperties: false, +}; +const isStateEventIdQueryProps = ajv.compile(StateEventIdQuerySchema); @@ - .get( - '/v1/state_ids/:roomId', - { - params: isGetStateIdsParamsProps, - response: { - 200: isGetStateIdsResponseProps, - }, - }, + .get( + '/v1/state_ids/:roomId', + { + params: isRoomIdParamsProps, + query: isStateEventIdQueryProps, + response: { + 200: isGetStateIdsResponseProps, + }, + }, async (c) => { const roomId = c.req.param('roomId'); const eventId = c.req.query('event_id'); @@ - .get( - '/v1/state/:roomId', - { - params: isGetStateParamsProps, - response: { - 200: isGetStateResponseProps, - }, - }, + .get( + '/v1/state/:roomId', + { + params: isRoomIdParamsProps, + query: isStateEventIdQueryProps, + response: { + 200: isGetStateResponseProps, + }, + }, async (c) => { const roomId = c.req.param('roomId'); const eventId = c.req.query('event_id');ee/packages/federation-matrix/src/FederationMatrix.ts (2)
118-118: Bug: enableThumbnails is always true due to|| trueThis ignores the env setting; thumbnails can’t be disabled.
Apply:
- enableThumbnails: process.env.MEDIA_ENABLE_THUMBNAILS === 'true' || true, + // default true; allow explicit 'false' to disable + enableThumbnails: process.env.MEDIA_ENABLE_THUMBNAILS + ? process.env.MEDIA_ENABLE_THUMBNAILS === 'true' + : true,
392-415: Critical: Wrong federation ID used when ensuring DM members existYou’re querying/creating users using the local
_idasfederation.mui.muimust be the Matrix user ID (e.g.,@user:domain). This will corrupt user records and create duplicates.Use the Matrix ID for lookup/creation:
- if (memberId) { - const existingMemberMatrixUserId = await Users.findOne({ 'federation.mui': memberId }); - if (!existingMemberMatrixUserId) { - const newUser = { - username: memberId, - name: memberId, - type: 'user' as const, - status: UserStatus.OFFLINE, - active: true, - roles: ['user'], - requirePasswordChange: false, - federated: true, - federation: { - version: 1, - mui: memberId, - origin: memberMatrixUserId.split(':').pop(), - }, - createdAt: new Date(), - _updatedAt: new Date(), - }; - - await Users.insertOne(newUser); - } - } + // Ensure the remote Matrix user exists locally by matrix user id (mui) + if (memberMatrixUserId) { + const existingRemoteUser = await Users.findOne({ 'federation.mui': memberMatrixUserId }, { projection: { _id: 1 } }); + if (!existingRemoteUser) { + const localNameFromMui = memberMatrixUserId.split(':')[0]?.replace('@', '') || memberMatrixUserId; + const newUser = { + username: localNameFromMui, + name: localNameFromMui, + type: 'user' as const, + status: UserStatus.OFFLINE, + active: true, + roles: ['user'], + requirePasswordChange: false, + federated: true, + federation: { + version: 1, + mui: memberMatrixUserId, + origin: memberMatrixUserId.split(':').pop(), + }, + createdAt: new Date(), + _updatedAt: new Date(), + }; + await Users.insertOne(newUser); + } + }ee/packages/federation-matrix/src/api/_matrix/invite.ts (1)
137-150: Critical: Backoff logic mixes seconds and milliseconds; can explode or retry too fast
delaySecis squared and logged as ms; also passed incorrectly tosetTimeout, causing erratic behavior.Use consistent seconds and cap properly:
-async function runWithBackoff(fn: () => Promise<void>, delaySec = 5) { - try { - await fn(); - } catch (e) { - const delay = delaySec === 625 ? 625 : delaySec ** 2; - console.log(`error occurred, retrying in ${delay}ms`, e); - setTimeout(() => { - runWithBackoff(fn, delay * 1000); - }, delay); - } -} +async function runWithBackoff(fn: () => Promise<void>, delaySec = 5) { + try { + await fn(); + } catch (e) { + const nextDelaySec = Math.min(delaySec ** 2, 625); + console.log(`error occurred, retrying in ${nextDelaySec}s`, e); + setTimeout(() => { + void runWithBackoff(fn, nextDelaySec); + }, nextDelaySec * 1000); + } +}
🧹 Nitpick comments (5)
ee/packages/federation-matrix/src/events/room.ts (1)
7-58: Avoid throwing in async event handlers; wrap with try/catch and logUncaught throws inside emitter callbacks can surface as unhandled rejections. Prefer try/catch with logging (as you do in other event handlers).
Example for the first handler:
emitter.on('homeserver.matrix.room.name', async (data) => { - const { room_id: roomId, name, user_id: userId } = data; + try { + const { room_id: roomId, name, user_id: userId } = data; - const localRoomId = await Rooms.findOne({ 'federation.mrid': roomId }, { projection: { _id: 1 } }); - if (!localRoomId) { - throw new Error('mapped room not found'); - } + const localRoomId = await Rooms.findOne({ 'federation.mrid': roomId }, { projection: { _id: 1 } }); + if (!localRoomId) { + throw new Error('mapped room not found'); + } - const localUserId = await Users.findOne({ 'federation.mui': userId }, { projection: { _id: 1 } }); - if (!localUserId) { - throw new Error('mapped user not found'); - } + const localUserId = await Users.findOne({ 'federation.mui': userId }, { projection: { _id: 1 } }); + if (!localUserId) { + throw new Error('mapped user not found'); + } - await Room.saveRoomName(localRoomId._id, localUserId._id, name); + await Room.saveRoomName(localRoomId._id, localUserId._id, name); + } catch (e) { + // consider a scoped logger here for consistency with other files + // logger.error('Failed to handle matrix.room.name event', e); + throw e; // or swallow if desired behavior + } });ee/packages/federation-matrix/src/api/_matrix/send-join.ts (1)
40-47: Optional: Rename “stateKey” param to “eventId” for clarityIn Matrix, the second path segment is an event ID. Renaming improves readability without changing behavior.
-const SendJoinParamsSchema = { +const SendJoinParamsSchema = { type: 'object', properties: { roomId: RoomIdSchema, - stateKey: EventIdSchema, + eventId: EventIdSchema, }, - required: ['roomId', 'stateKey'], + required: ['roomId', 'eventId'], }; ... - return new Router('/federation').put( - '/v2/send_join/:roomId/:stateKey', + return new Router('/federation').put( + '/v2/send_join/:roomId/:eventId', { params: isSendJoinParamsProps, body: isSendJoinEventProps, ... - const { roomId, stateKey } = c.req.param(); + const { roomId, eventId } = c.req.param(); const body = await c.req.json(); - const response = await sendJoin.sendJoin(roomId, stateKey as EventID, body); + const response = await sendJoin.sendJoin(roomId, eventId as EventID, body);Also applies to: 227-244
ee/packages/federation-matrix/src/events/invite.ts (1)
11-12: Prefer Logger over console for consistency and structured logsOther handlers use @rocket.chat/logger; align this file too.
import { Rooms, Users } from '@rocket.chat/models'; +import { Logger } from '@rocket.chat/logger'; +const logger = new Logger('federation-matrix:invite'); @@ - console.warn(`No bridged room found for room_id: ${data.room_id}`); + logger.warn(`No bridged room found for room_id: ${data.room_id}`); @@ - console.warn(`User with ID ${insertedId} not found after insertion`); + logger.warn(`User with ID ${insertedId} not found after insertion`);Also applies to: 47-48
ee/packages/federation-matrix/src/events/ping.ts (1)
2-2: SDK import switch looks good.Type-only import from the published SDK is correct. Consider replacing the
console.logon Line 6 with our standard logger for consistency and observability.ee/packages/federation-matrix/src/events/message.ts (1)
40-41: attachments type should be an array, not a 1‑tuple.Using
[FileAttachmentProps]creates a fixed-length tuple. PreferFileAttachmentProps[].- attachments: [FileAttachmentProps]; + attachments: FileAttachmentProps[];
📜 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 ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (23)
ee/apps/federation-service/package.json(1 hunks)ee/packages/federation-matrix/package.json(1 hunks)ee/packages/federation-matrix/src/FederationMatrix.ts(2 hunks)ee/packages/federation-matrix/src/api/.well-known/server.ts(1 hunks)ee/packages/federation-matrix/src/api/_matrix/invite.ts(1 hunks)ee/packages/federation-matrix/src/api/_matrix/key/server.ts(1 hunks)ee/packages/federation-matrix/src/api/_matrix/media.ts(1 hunks)ee/packages/federation-matrix/src/api/_matrix/profiles.ts(1 hunks)ee/packages/federation-matrix/src/api/_matrix/rooms.ts(1 hunks)ee/packages/federation-matrix/src/api/_matrix/send-join.ts(1 hunks)ee/packages/federation-matrix/src/api/_matrix/transactions.ts(1 hunks)ee/packages/federation-matrix/src/api/_matrix/versions.ts(1 hunks)ee/packages/federation-matrix/src/api/middlewares.ts(1 hunks)ee/packages/federation-matrix/src/events/edu.ts(1 hunks)ee/packages/federation-matrix/src/events/index.ts(1 hunks)ee/packages/federation-matrix/src/events/invite.ts(1 hunks)ee/packages/federation-matrix/src/events/member.ts(1 hunks)ee/packages/federation-matrix/src/events/message.ts(1 hunks)ee/packages/federation-matrix/src/events/ping.ts(1 hunks)ee/packages/federation-matrix/src/events/reaction.ts(1 hunks)ee/packages/federation-matrix/src/events/room.ts(1 hunks)ee/packages/federation-matrix/src/services/MatrixMediaService.ts(1 hunks)package.json(0 hunks)
💤 Files with no reviewable changes (1)
- package.json
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: rodrigok
PR: RocketChat/Rocket.Chat#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.
📚 Learning: 2025-09-19T15:15:04.642Z
Learnt from: rodrigok
PR: RocketChat/Rocket.Chat#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/src/api/_matrix/rooms.tsee/packages/federation-matrix/package.jsonee/packages/federation-matrix/src/api/.well-known/server.tsee/packages/federation-matrix/src/api/_matrix/key/server.tsee/packages/federation-matrix/src/api/_matrix/transactions.tsee/packages/federation-matrix/src/events/reaction.tsee/packages/federation-matrix/src/events/message.tsee/packages/federation-matrix/src/api/_matrix/versions.tsee/packages/federation-matrix/src/services/MatrixMediaService.tsee/packages/federation-matrix/src/events/member.tsee/apps/federation-service/package.jsonee/packages/federation-matrix/src/events/room.tsee/packages/federation-matrix/src/api/_matrix/invite.tsee/packages/federation-matrix/src/api/_matrix/profiles.tsee/packages/federation-matrix/src/api/_matrix/send-join.tsee/packages/federation-matrix/src/api/_matrix/media.tsee/packages/federation-matrix/src/FederationMatrix.tsee/packages/federation-matrix/src/events/ping.tsee/packages/federation-matrix/src/events/edu.tsee/packages/federation-matrix/src/events/invite.tsee/packages/federation-matrix/src/events/index.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). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (21)
ee/packages/federation-matrix/src/api/_matrix/rooms.ts (1)
1-1: Import updated to published SDK — looks goodNo behavior change; types sourced from the external package as intended.
ee/packages/federation-matrix/src/events/edu.ts (1)
4-4: Import updated to published SDK — looks goodType source switch only; event handling unchanged.
ee/packages/federation-matrix/src/events/room.ts (1)
3-3: Import updated to published SDK — looks goodType-only import swap; local logic untouched.
ee/packages/federation-matrix/src/api/_matrix/send-join.ts (1)
1-1: Import updated to published SDK — looks goodBringing EventID together with HomeserverServices from the new SDK is correct.
ee/packages/federation-matrix/src/api/_matrix/transactions.ts (1)
1-1: Import updated to published SDK — looks goodNo runtime change; aligns with the migration.
ee/packages/federation-matrix/src/events/invite.ts (1)
4-4: Import updated to published SDK — looks goodType-only change; behavior intact.
ee/packages/federation-matrix/src/events/reaction.ts (1)
3-3: Import updated to published SDK — looks goodType import source swap only; logic remains the same.
ee/packages/federation-matrix/src/events/member.ts (1)
3-3: Import updated to published SDK — looks goodType-only import swap is correct and aligns with the PR objective. Automated search was inconclusive (ripgrep skipped files); run a repo-wide check and confirm no @hs/* imports remain: git grep -n "@hs/" || rg -n --hidden --no-ignore "@hs/" --glob '!/node_modules/'
ee/packages/federation-matrix/src/events/message.ts (1)
4-4: Consolidated type imports to published SDK — looks good.Purely type-level; no runtime impact.
ee/packages/federation-matrix/src/api/_matrix/profiles.ts (1)
1-1: Type import source updated — OK.No signature/runtime changes; aligned with the published SDK.
ee/packages/federation-matrix/src/api/middlewares.ts (1)
1-2: errCodes and types sourced from published SDK — OK.Assuming
errorCodekeys are unchanged, responses remain consistent.Please confirm at least one negative path returns the expected
errcodeand status with the new SDK during E2E/API tests.ee/packages/federation-matrix/src/api/_matrix/key/server.ts (1)
1-1: Type import source updated — OK.No behavior change.
ee/packages/federation-matrix/package.json (1)
40-40: Added published SDK dependency — OK.Version matches federation-service. Keep these in sync going forward.
ee/packages/federation-matrix/src/api/.well-known/server.ts (1)
5-5: Type import source updated — OK.No functional change; TODOs remain out of scope for this PR.
ee/apps/federation-service/package.json (1)
28-28: Dependency moved to published SDK — OK.Pinned to 0.1.3 is fine; repo search shows "@rocket.chat/federation-sdk" at 0.1.3 in package.json/yarn.lock and no
@hs/*references remain.ee/packages/federation-matrix/src/events/index.ts (1)
2-2: Type import migration LGTMImporting
HomeserverEventSignaturesfrom the new SDK aligns with the rest of the PR.ee/packages/federation-matrix/src/api/_matrix/invite.ts (1)
3-10: SDK type import updates are correctAll types moved to
@rocket.chat/federation-sdkwith no signature changes. Looks good.ee/packages/federation-matrix/src/services/MatrixMediaService.ts (1)
3-3: SDK type import migration LGTMNo behavior changes; service wiring remains intact.
ee/packages/federation-matrix/src/api/_matrix/media.ts (1)
4-4: SDK type import migration LGTMRoutes continue to accept
HomeserverServicesfrom the new SDK.ee/packages/federation-matrix/src/api/_matrix/versions.ts (1)
1-1: SDK type import migration LGTMSimple import source switch; endpoint behavior unchanged.
ee/packages/federation-matrix/src/FederationMatrix.ts (1)
14-22: SDK import migration to @rocket.chat/federation-sdk looks good — verification required
Automated search ran but many tracked files were missing in the sandbox (ripgrep reported multiple "No such file or directory" errors), so results are inconclusive; confirm locally that there are no remaining @hs/* imports.
| }; | ||
|
|
||
| export { generateEd25519RandomSecretKey } from '@hs/crypto'; | ||
| export { generateEd25519RandomSecretKey } from '@rocket.chat/federation-sdk'; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify exported symbol availability in the new SDK
Please confirm generateEd25519RandomSecretKey is exported by @rocket.chat/federation-sdk@0.1.3.
🌐 Web query:
Does @rocket.chat/federation-sdk@0.1.3 export a `generateEd25519RandomSecretKey` function?
💡 Result:
I couldn't find any export named generateEd25519RandomSecretKey in @rocket.chat/federation-sdk@0.1.3. (I searched the Rocket.Chat federation repositories and npm-related package listings and found no reference to that function.) [1][2]
If you want, I can fetch the package tarball (or a specific file path in that version) and inspect its exports directly.
Fix broken re-export — missing symbol in SDK
generateEd25519RandomSecretKey is not exported by @rocket.chat/federation-sdk@0.1.3; the re-export in ee/packages/federation-matrix/src/FederationMatrix.ts:51 will fail. Replace the import with the actual exported symbol from the SDK, vendor/implement the function locally, or pin/revert to an SDK version that provides it.
🤖 Prompt for AI Agents
In ee/packages/federation-matrix/src/FederationMatrix.ts around line 51, the
file re-exports generateEd25519RandomSecretKey which is not exported by
@rocket.chat/federation-sdk@0.1.3 and will break the build; remove this broken
re-export and either (a) replace it with the actual symbol name exported by the
SDK (update the import to the correct exported function if the SDK provides it),
(b) vendor/implement a local generateEd25519RandomSecretKey in this package and
export that instead, or (c) pin/revert the dependency to a SDK version that
includes this symbol; ensure whichever approach you choose updates
imports/exports and tests accordingly so no unresolved export remains.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
.github/workflows/ci-test-storybook.yml (4)
13-16: Pass NPM_TOKEN to the reusable workflow (currently missing).This workflow references secrets.NPM_TOKEN in the setup step but the reusable workflow does not declare/receive it, so it will be empty when called and can break installs that require auth.
Apply:
workflow_call: @@ - secrets: - CODECOV_TOKEN: - required: true + secrets: + CODECOV_TOKEN: + required: true + NPM_TOKEN: + required: falseAlso ensure the caller forwards NPM_TOKEN in the calling workflow’s
secretsmapping. Would you like me to draft that change too?
40-45: Make turbo-build artifact restore optional or non-fatal.If no upstream job uploads “turbo-build”, this step will hard-fail. Given you already use turbo cache, consider gating or softening this.
Preferred:
- - name: Restore turbo build - uses: actions/download-artifact@v4 - with: - name: turbo-build - path: .turbo/cache + - name: Restore turbo build + if: ${{ inputs.restore-turbo-build }} + uses: actions/download-artifact@v4 + with: + name: turbo-build + path: .turbo/cache + continue-on-error: trueAnd add an input:
workflow_call: inputs: node-version: required: true type: string deno-version: required: true type: string + restore-turbo-build: + required: false + type: boolean + default: falseIf you prefer, you can also just remove the restore step and rely on the turbo cache.
29-37: Setup NodeJS step references NPM_TOKEN; ensure it’s actually available.After adding NPM_TOKEN to
workflow_call.secrets, thewith: NPM_TOKEN: ${{ secrets.NPM_TOKEN }}here will work as intended. Without that, this may fail on private deps.
53-57: Upgrade codecov/codecov-action to @v5 and update inputsCodecov now recommends codecov/codecov-action@v5; v4 introduced breaking input changes (token required by default, moved to the Codecov CLI, renamed/removed inputs — e.g., file→files, plugin→plugins — and added working-directory). Update .github/workflows/ci-test-storybook.yml (currently @V3) and ensure token/flags/verbose are mapped correctly; I can provide the exact diff for @v5 if you want it.
📜 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 (6)
.github/workflows/ci-code-check.yml(0 hunks).github/workflows/ci-test-e2e.yml(0 hunks).github/workflows/ci-test-storybook.yml(1 hunks).github/workflows/ci-test-unit.yml(0 hunks).github/workflows/ci.yml(3 hunks).gitignore(0 hunks)
💤 Files with no reviewable changes (4)
- .github/workflows/ci-code-check.yml
- .github/workflows/ci-test-unit.yml
- .github/workflows/ci-test-e2e.yml
- .gitignore
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: rodrigok
PR: RocketChat/Rocket.Chat#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.
⏰ 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)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (4)
.github/workflows/ci-test-storybook.yml (1)
48-52: Sanity check: homeserver artifact removal.Since the homeserver artifact and symlink were removed elsewhere, confirm Storybook tests don’t rely on any /tmp/homeserver paths or env. Looks clean here.
.github/workflows/ci.yml (3)
191-191: Build job dependency update: OK — meteor-build contains no homeserver/artifact references.
Inspected .github/actions/meteor-build/action.yml and searched that action for "homeserver", "download-artifact"/"actions/download-artifact", "artifact.*home", and related patterns — no matches found.
297-297: Docker coverage build deps: OK — workflows do not download/link a homeserver artifact.
Scanned .github/workflows and the repo: download/upload-artifact steps reference 'turbo-build', 'build', 'playwright-test-trace' and coverage artifacts; no 'homeserver' or 'build-homeserver' artifact references found.
138-138: Remove homeserver dependency — no CI/workflow references found.Searched .github/workflows and .github/actions for "build-homeserver" / "homeserver" — no matches; remaining occurrences are runtime/app-level (ee/packages/federation-matrix, packages/i18n, tests) and don’t indicate a CI artifact dependency.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## chore/federation-backup #37046 +/- ##
===========================================================
- Coverage 69.75% 69.72% -0.03%
===========================================================
Files 3035 3035
Lines 103431 103431
Branches 18383 18377 -6
===========================================================
- Hits 72146 72116 -30
- Misses 29405 29436 +31
+ Partials 1880 1879 -1
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 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 ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (1)
apps/meteor/package.json(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: rodrigok
PR: RocketChat/Rocket.Chat#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.
⏰ 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)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
| "tsyringe": "^4.10.0", | ||
| "tweetnacl": "^1.0.3", |
There was a problem hiding this comment.
🧩 Analysis chain
Verify DI setup and TS typings for the new deps (tsyringe, tweetnacl).
- Ensure reflect-metadata is imported before any tsyringe usage on both client and server entrypoints; the dependency exists (Line 439), but initialization import is required.
- Confirm tsconfig enables experimentalDecorators and emitDecoratorMetadata for any project using tsyringe.
- If tweetnacl is imported directly in TS code, add @types/tweetnacl to devDependencies (unless the SDK wraps usage and provides types).
- Validate whether these are needed here vs. only in the packages that directly consume them, unless the SDK lists them as peerDependencies for the app.
Run this script to verify usage, config, and typing needs:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Search for direct imports of tsyringe and tweetnacl in TS files =="
rg -nP --type=ts --type=tsx -C2 "(from\s+['\"]tsyringe['\"]|from\s+['\"]tweetnacl['\"]|require\(['\"]tsyringe['\"]\)|require\(['\"]tweetnacl['\"]\))"
echo
echo "== Check that reflect-metadata is imported early in app entrypoints =="
rg -nP -C2 "import\s+['\"]reflect-metadata['\"]" apps/meteor/server/main.ts apps/meteor/client/main.ts || true
echo
echo "== Verify tsconfig decorator settings across workspaces =="
fd tsconfig.json | while read -r f; do
echo "-- $f"
jq '{file:$ENV.f, experimentalDecorators: .compilerOptions.experimentalDecorators, emitDecoratorMetadata: .compilerOptions.emitDecoratorMetadata}' "$f" 2>/dev/null || true
done
echo
echo "== Check for @types/tweetnacl presence =="
rg -nP '"@types/tweetnacl"\s*:' -C1 package.json **/package.json || true
echo
echo "== Inspect locked versions to avoid duplicates =="
rg -n "tsyringe@|tweetnacl@" yarn.lock -n -C1 || true
echo
echo "== Inspect federation SDK peerDependencies (if vendored in monorepo) =="
fd package.json | rg -nP -C3 '"name"\s*:\s*"@rocket\.chat/federation-sdk"|peerDependencies|dependencies' || trueLength of output: 97
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Search for direct imports of tsyringe and tweetnacl in .ts/.tsx files =="
rg -nP -g '**/*.ts' -g '**/*.tsx' -C2 "(from\s+['\"]tsyringe['\"]|from\s+['\"]tweetnacl['\"]|require\(['\"]tsyringe['\"]\)|require\(['\"]tweetnacl['\"]\))" || true
echo
echo "== Check that reflect-metadata is imported early in apps/meteor entrypoints =="
rg -nP "import\s+['\"]reflect-metadata['\"]" apps/meteor -S -n -C2 || true
echo
echo "== Verify tsconfig decorator settings across workspaces =="
find . -name tsconfig.json -not -path '*/node_modules/*' -print0 | while IFS= read -r -d '' f; do
echo "-- $f"
jq '{experimentalDecorators: .compilerOptions.experimentalDecorators, emitDecoratorMetadata: .compilerOptions.emitDecoratorMetadata}' "$f" 2>/dev/null || echo " (invalid json or missing compilerOptions)"
done
echo
echo "== Check for @types/tweetnacl presence =="
rg -nP '"@types/tweetnacl"' -S || true
echo
echo "== Inspect yarn.lock for tsyringe@ or tweetnacl@ =="
if [ -f yarn.lock ]; then
rg -n "tsyringe@|tweetnacl@" yarn.lock -n -C1 || true
else
echo "yarn.lock not found"
fi
echo
echo "== Dump package.json name/peerDependencies/dependencies for review =="
find . -name package.json -not -path '*/node_modules/*' -print0 | while IFS= read -r -d '' f; do
echo "-- $f"
jq '{name: .name, peerDependencies: .peerDependencies, dependencies: .dependencies}' "$f" 2>/dev/null || echo " (invalid json)"
doneLength of output: 49309
Action required — fix DI import, TS decorator settings, and typings for tsyringe / tweetnacl
- reflect-metadata not imported early: no
import 'reflect-metadata'found in apps/meteor scans — add it at the top of app entrypoints (apps/meteor server & client entrypoints) and in the entrypoints of packages that declare tsyringe (ee/apps/federation-service, ee/packages/federation-matrix) before any decorator/DI usage. - TS compiler flags inconsistent: many tsconfig.json files lack the required flags. Ensure any package that consumes tsyringe has compilerOptions.experimentalDecorators = true and compilerOptions.emitDecoratorMetadata = true. Example: ./ee/packages/federation-matrix/tsconfig.json has experimentalDecorators=true but emitDecoratorMetadata is missing — enable it. (./packages/apps-engine/tsconfig.json already has both set.)
- Missing types for tweetnacl: no @types/tweetnacl found — if tweetnacl is imported from TypeScript, add @types/tweetnacl to the consuming package(s) devDependencies (apps/meteor, ee/apps/federation-service, ee/packages/federation-matrix) or to repo devDeps.
- Duplicate versions in lockfile: yarn.lock contains tweetnacl@1.0.3 and 0.14.5 — dedupe/align versions or add resolutions to avoid multiple bundles.
- Usage vs placement: scan found no direct .ts/.tsx imports of tsyringe/tweetnacl — confirm they’re actually used by the app level; if only SDKs/packages use them, move the deps to those packages or declare them as peerDependencies.
🤖 Prompt for AI Agents
In apps/meteor/package.json around lines 454-455, address DI and typing issues:
add import 'reflect-metadata' at the very top of the server and client
entrypoints (and at the top of any package entrypoints that use decorators or
tsyringe such as ee/apps/federation-service and ee/packages/federation-matrix)
before any decorator/DI usage; ensure each package that consumes tsyringe sets
compilerOptions.experimentalDecorators = true and
compilerOptions.emitDecoratorMetadata = true in its tsconfig.json (enable
emitDecoratorMetadata in ./ee/packages/federation-matrix/tsconfig.json); add
@types/tweetnacl to the devDependencies of consuming packages (apps/meteor,
ee/apps/federation-service, ee/packages/federation-matrix) or to repo devDeps if
preferred; resolve duplicate tweetnacl versions in yarn.lock by aligning
versions or adding a yarn/npm resolution so only one version is installed;
finally verify whether tsyringe/tweetnacl are used at app-level—if only
SDKs/packages use them move the dependencies into those packages or mark them as
peerDependencies at the app level.
Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Further comments
Summary by CodeRabbit