Skip to content

Conversation

@sampaiodiego
Copy link
Member

@sampaiodiego sampaiodiego commented Oct 31, 2025

FDR-156

Summary by CodeRabbit

  • New Features

    • Introduced a single init() bootstrap and a federationSDK singleton as the primary runtime entrypoint.
  • Refactor

    • Consolidated many federation operations behind the new SDK facade.
    • Reworked initialization to use runtime init and collection-oriented setup instead of prior container wiring.
    • Homeserver setup simplified; now returns only the app.
  • Style / API

    • Simplified middleware signatures (no service parameters).
    • Config API unified to getConfig(...) and added edu settings for typing/presence.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 31, 2025

Walkthrough

Replaces DI/container and module exports with a singleton FederationSDK facade and init bootstrap. Removes container.ts and federation.module.ts, consolidates service usage behind federationSDK, updates init/setConfig flows, and updates controllers and middlewares to call the SDK.

Changes

Cohort / File(s) Summary
Core SDK & Entry
packages/federation-sdk/src/container.ts, packages/federation-sdk/src/federation.module.ts, packages/federation-sdk/src/sdk.ts, packages/federation-sdk/src/index.ts
Removed DI container and NestJS module; added FederationSDK singleton facade in sdk.ts; replaced many individual exports with an init(...) bootstrap and federationSDK instance.
Config & DB Connection
packages/federation-sdk/src/services/config.service.ts, packages/federation-sdk/src/services/database-connection.service.ts
ConfigService now uses setConfig() and getConfig<K>() (added @singleton); DatabaseConnectionService constructor changed to accept a plain DB config object instead of ConfigService.
Service Adjustments
packages/federation-sdk/src/services/federation-request.service.ts, packages/federation-sdk/src/services/federation.service.ts, packages/federation-sdk/src/services/invite.service.ts
FederationRequestService now reads serverName via getConfig('serverName'); FederationService lazily injects StateService; InviteService retrieves invite config via getConfig('invite').
StateService Change
packages/federation-sdk/src/services/state.service.ts
Removed EventRepository parameter from StateService constructor.
Tests Updated
packages/federation-sdk/src/services/*/*.spec.ts, packages/federation-sdk/src/services/state.service.spec.ts, packages/federation-sdk/src/services/federation-request.service.spec.ts
Tests updated to use new DatabaseConnectionService constructor shape and ConfigService.getConfig() mock method.
Homeserver Controllers (federation)
packages/homeserver/src/controllers/federation/{invite,media,profiles,rooms,send-join,state,transactions,versions}.controller.ts
Replaced per-service DI/container.resolve usages with federationSDK.* calls; simplified middleware invocations (removed service args).
Homeserver Controllers (internal)
packages/homeserver/src/controllers/internal/{direct-message,external-federation-request,invite,message,room}.controller.ts
Replaced Room/Message/State/Federation service resolutions with federationSDK calls; updated call sites accordingly.
Key & Well-Known Controllers
packages/homeserver/src/controllers/key/server.controller.ts, packages/homeserver/src/controllers/well-known/well-known.controller.ts
Switched ServerService/WellKnownService usage to federationSDK.getSignedServerKey() / federationSDK.getWellKnownHostData().
Middlewares
packages/homeserver/src/middlewares/{isAuthenticated,canAccessResource}.ts
Removed EventAuthorizationService parameter; middleware signatures now parameterless or accept only resource type and call federationSDK.verifyRequestSignature() / federationSDK.canAccessResource().
Homeserver Setup
packages/homeserver/src/homeserver.module.ts
Removed federation container/bootstrap; setup() now calls init() and federationSDK.setConfig(); setup() signature simplified to parameterless and now returns { app } (no container).

Sequence Diagram(s)

sequenceDiagram
    participant App as Application
    participant Init as init(dbConfig?, emitter?)
    participant SDK as federationSDK (singleton)
    participant Service as Underlying services

    App->>Init: call init(...)
    Init->>SDK: register collections, attach emitter, setConfig
    Init-->>App: ready federationSDK

    Note over SDK,Service: runtime delegation
    App->>SDK: federationSDK.processInvite(...)
    SDK->>Service: delegate to InviteService implementation
    Service-->>SDK: result
    SDK-->>App: result
Loading
sequenceDiagram
    participant Controller as Controller
    participant Old as Container.resolve(Service)
    participant New as federationSDK

    Controller->>Old: container.resolve(InviteService)
    Old-->>Controller: InviteService instance
    Controller->>Old: inviteService.processInvite(...)

    Controller->>New: federationSDK.processInvite(...)
    New-->>Controller: result
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Areas needing extra attention:
    • init() collection registration, DB wiring, and StagingAreaListener lifecycle.
    • Correctness of FederationSDK method delegations and lazy injections.
    • All controller/middleware signature changes (ensure no missing parameters).
    • ConfigService key names usage (getConfig('...')) across callers.
    • DatabaseConnectionService constructor usage across tests and init flow.

Possibly related PRs

Suggested reviewers

  • ggazzo

Poem

🐰 I boxed the container, hopped with glee,
One SDK to call, neat as can be.
Services wrapped in a single-knit seam,
Controllers dancing in a simpler dream.
Thump-thump — refactor carrots for tea! 🥕

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title 'feat: introduce FederationSDK class' directly and accurately describes the main change in the changeset. The PR introduces a new FederationSDK class (in packages/federation-sdk/src/sdk.ts) that acts as a high-level facade for federation-related services, which is clearly the primary architectural focus of this comprehensive refactoring. The title is concise, clear, and specific enough for a teammate scanning the history to understand the central change.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat-create-sdk-class

📜 Recent 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 9408507 and 126d650.

📒 Files selected for processing (4)
  • packages/federation-sdk/src/services/state.service.spec.ts (1 hunks)
  • packages/federation-sdk/src/services/state.service.ts (0 hunks)
  • packages/homeserver/src/controllers/internal/invite.controller.ts (3 hunks)
  • packages/homeserver/src/homeserver.module.ts (3 hunks)
💤 Files with no reviewable changes (1)
  • packages/federation-sdk/src/services/state.service.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/homeserver/src/controllers/internal/invite.controller.ts
  • packages/federation-sdk/src/services/state.service.spec.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/homeserver/src/homeserver.module.ts (1)
packages/federation-sdk/src/index.ts (2)
  • init (264-323)
  • federationSDK (325-325)
🔇 Additional comments (2)
packages/homeserver/src/homeserver.module.ts (2)

128-128: No breaking changes detected—removal of container from return value is safe.

The setup() function is only called once internally (line 131) where it already destructures only { app }. The only external export is appPromise, which also only uses app. Controllers that need the DI container import it directly from tsyringe, not from the setup function's return value. The change aligns with hiding implementation details without breaking any dependencies.


49-49: The original concerns are incorrect and should not be addressed.

The instanceId is used exclusively for distributed locking, not federation identity or event signatures. The instanceId is stored in lock records along with roomId and timestamps, but these locks have a 2-minute timeout. The fresh UUID generated on each startup is intentional and correct: when an instance crashes, its lock expires after 2 minutes, allowing another instance to acquire the lock with its own instanceId. Additionally, setup() is called once at module initialization, not multiple times, so the premise of repeated setup calls is also unfounded.

Likely an incorrect or invalid review comment.


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

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

@codecov-commenter
Copy link

codecov-commenter commented Oct 31, 2025

Codecov Report

❌ Patch coverage is 53.33333% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.47%. Comparing base (ef067a1) to head (126d650).

Files with missing lines Patch % Lines
...on-sdk/src/services/database-connection.service.ts 0.00% 4 Missing ⚠️
...ages/federation-sdk/src/services/config.service.ts 70.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #292      +/-   ##
==========================================
- Coverage   60.50%   60.47%   -0.03%     
==========================================
  Files          67       67              
  Lines        6676     6670       -6     
==========================================
- Hits         4039     4034       -5     
+ Misses       2637     2636       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
packages/homeserver/src/controllers/internal/message.controller.ts (1)

147-177: Missing error handling for redactMessage endpoint.

Unlike the other three endpoints in this file, the redactMessage handler does not wrap the federationSDK.redactMessage call in a try-catch block. If the SDK method throws an error, it will propagate as an unhandled exception rather than returning the expected ErrorResponse with a 500 status.

Apply this diff to add consistent error handling:

 		.delete(
 			'/internal/messages/:messageId',
 			async ({
 				params,
 				body,
+				set,
 			}): Promise<InternalRedactMessageResponse | ErrorResponse> => {
 				const { roomId } = body;
-				const eventId = await federationSDK.redactMessage(
-					roomId as RoomID,
-					params.messageId as EventID,
-				);
+				try {
+					const eventId = await federationSDK.redactMessage(
+						roomId as RoomID,
+						params.messageId as EventID,
+					);
 
-				return {
-					event_id: eventId,
-					origin_server_ts: Date.now(),
-				};
+					return {
+						event_id: eventId,
+						origin_server_ts: Date.now(),
+					};
+				} catch (error) {
+					set.status = 500;
+					return {
+						error: `Failed to redact message: ${error instanceof Error ? error.message : String(error)}`,
+						details: {},
+					};
+				}
 			},
packages/homeserver/src/controllers/internal/invite.controller.ts (1)

20-28: Remove commented-out code.

This commented-out code block appears to be leftover from a previous implementation and should be removed. Version control preserves the history if needed.

Apply this diff to remove the dead code:

 		async ({ body }): Promise<InternalInviteUserResponse | ErrorResponse> => {
-			// try {
-			// 	return inviteService.inviteUserToRoom(username, roomId, sender, name);
-			// } catch (error) {
-			// 	set.status = 500;
-			// 	return {
-			// 		error: `Failed to invite user: ${error instanceof Error ? error.message : String(error)}`,
-			// 		details: {},
-			// 	};
-			// }
 			const { roomId, username, sender } = body;
packages/homeserver/src/homeserver.module.ts (2)

53-103: setConfig throws because edu is missing

AppConfigSchema now requires the edu block, but this call omits it. At runtime the Zod parse fails with “edu: Required” and the homeserver never boots. Please supply the new config (with sensible defaults and env overrides).

 	federationSDK.setConfig({
 		instanceId: crypto.randomUUID(),
 		serverName: process.env.SERVER_NAME || 'rc1',
 		port: Number.parseInt(process.env.SERVER_PORT || '8080', 10),
 		matrixDomain: process.env.MATRIX_DOMAIN || 'rc1',
 		keyRefreshInterval: Number.parseInt(
 			process.env.MATRIX_KEY_REFRESH_INTERVAL || '60',
 			10,
 		),
 		signingKey: process.env.SIGNING_KEY,
 		signingKeyPath: process.env.CONFIG_FOLDER || './rc1.signing.key',
 		version: process.env.SERVER_VERSION || '1.0',
 		media: {
 …
 		},
 		invite: {
 			allowedEncryptedRooms:
 				process.env.INVITE_ALLOWED_ENCRYPTED_ROOMS === 'true',
 			allowedNonPrivateRooms:
 				process.env.INVITE_ALLOWED_NON_PRIVATE_ROOMS === 'true',
 		},
+		edu: {
+			processTyping:
+				process.env.EDU_PROCESS_TYPING === undefined
+					? true
+					: process.env.EDU_PROCESS_TYPING === 'true',
+			processPresence:
+				process.env.EDU_PROCESS_PRESENCE === undefined
+					? true
+					: process.env.EDU_PROCESS_PRESENCE === 'true',
+		},
 	});

71-95: Fix enableThumbnails boolean coercion

process.env.MEDIA_ENABLE_THUMBNAILS === 'true' || true always evaluates to true, so operators can’t disable thumbnails even when they set the env to 'false'. Please gate the default explicitly.

-			enableThumbnails: process.env.MEDIA_ENABLE_THUMBNAILS === 'true' || true,
+			enableThumbnails:
+				process.env.MEDIA_ENABLE_THUMBNAILS === undefined
+					? true
+					: process.env.MEDIA_ENABLE_THUMBNAILS === 'true',
🧹 Nitpick comments (3)
packages/homeserver/src/controllers/internal/room.controller.ts (1)

4-4: Remove unused import.

The container import from tsyringe is no longer used after migrating to federationSDK.

Apply this diff:

-import { container } from 'tsyringe';
packages/homeserver/src/controllers/internal/direct-message.controller.ts (1)

4-4: Remove unused import.

The container import from tsyringe is no longer used after migrating to federationSDK.

Apply this diff:

-import { container } from 'tsyringe';
packages/homeserver/src/controllers/federation/send-join.controller.ts (1)

24-24: Consider removing or typing the as any assertion.

The body as any type assertion on line 24 may hide type mismatches. Consider either:

  1. Defining the proper type for the body parameter in federationSDK.sendJoin()
  2. Using the existing SendJoinEventDto type more strictly
📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between ef067a1 and 9408507.

📒 Files selected for processing (29)
  • packages/federation-sdk/src/container.ts (0 hunks)
  • packages/federation-sdk/src/federation.module.ts (0 hunks)
  • packages/federation-sdk/src/index.ts (2 hunks)
  • packages/federation-sdk/src/sdk.ts (1 hunks)
  • packages/federation-sdk/src/services/config.service.ts (4 hunks)
  • packages/federation-sdk/src/services/database-connection.service.ts (2 hunks)
  • packages/federation-sdk/src/services/federation-request.service.spec.ts (1 hunks)
  • packages/federation-sdk/src/services/federation-request.service.ts (1 hunks)
  • packages/federation-sdk/src/services/federation.service.ts (2 hunks)
  • packages/federation-sdk/src/services/invite.service.ts (1 hunks)
  • packages/federation-sdk/src/services/state.service.spec.ts (1 hunks)
  • packages/homeserver/src/controllers/federation/invite.controller.ts (2 hunks)
  • packages/homeserver/src/controllers/federation/media.controller.ts (2 hunks)
  • packages/homeserver/src/controllers/federation/profiles.controller.ts (5 hunks)
  • packages/homeserver/src/controllers/federation/rooms.controller.ts (3 hunks)
  • packages/homeserver/src/controllers/federation/send-join.controller.ts (2 hunks)
  • packages/homeserver/src/controllers/federation/state.controller.ts (3 hunks)
  • packages/homeserver/src/controllers/federation/transactions.controller.ts (6 hunks)
  • packages/homeserver/src/controllers/federation/versions.controller.ts (1 hunks)
  • packages/homeserver/src/controllers/internal/direct-message.controller.ts (2 hunks)
  • packages/homeserver/src/controllers/internal/external-federation-request.controller.ts (8 hunks)
  • packages/homeserver/src/controllers/internal/invite.controller.ts (3 hunks)
  • packages/homeserver/src/controllers/internal/message.controller.ts (5 hunks)
  • packages/homeserver/src/controllers/internal/room.controller.ts (13 hunks)
  • packages/homeserver/src/controllers/key/server.controller.ts (1 hunks)
  • packages/homeserver/src/controllers/well-known/well-known.controller.ts (1 hunks)
  • packages/homeserver/src/homeserver.module.ts (4 hunks)
  • packages/homeserver/src/middlewares/canAccessResource.ts (3 hunks)
  • packages/homeserver/src/middlewares/isAuthenticated.ts (2 hunks)
💤 Files with no reviewable changes (2)
  • packages/federation-sdk/src/container.ts
  • packages/federation-sdk/src/federation.module.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-10T22:18:31.655Z
Learnt from: sampaiodiego
Repo: RocketChat/homeserver PR: 224
File: packages/federation-sdk/src/services/event-authorization.service.ts:261-268
Timestamp: 2025-10-10T22:18:31.655Z
Learning: In packages/federation-sdk/src/services/state.service.ts, the method `StateService.getLatestRoomState(roomId: string)` has return type `Promise<State>` and never returns undefined. If the state is not found, it throws an error with message "No state found for room ${roomId}" instead of returning undefined.

Applied to files:

  • packages/homeserver/src/controllers/internal/invite.controller.ts
  • packages/homeserver/src/controllers/federation/state.controller.ts
  • packages/homeserver/src/controllers/internal/external-federation-request.controller.ts
  • packages/homeserver/src/controllers/federation/profiles.controller.ts
  • packages/federation-sdk/src/services/federation.service.ts
  • packages/homeserver/src/controllers/internal/direct-message.controller.ts
  • packages/homeserver/src/controllers/federation/rooms.controller.ts
  • packages/homeserver/src/controllers/internal/room.controller.ts
  • packages/federation-sdk/src/index.ts
  • packages/federation-sdk/src/sdk.ts
🧬 Code graph analysis (22)
packages/homeserver/src/controllers/federation/media.controller.ts (2)
packages/homeserver/src/middlewares/isAuthenticated.ts (1)
  • isAuthenticatedMiddleware (4-69)
packages/homeserver/src/middlewares/canAccessResource.ts (1)
  • canAccessResourceMiddleware (25-69)
packages/homeserver/src/controllers/internal/invite.controller.ts (1)
packages/federation-sdk/src/index.ts (1)
  • federationSDK (325-325)
packages/federation-sdk/src/services/state.service.spec.ts (1)
packages/federation-sdk/src/services/database-connection.service.ts (1)
  • DatabaseConnectionService (4-73)
packages/homeserver/src/controllers/federation/state.controller.ts (2)
packages/homeserver/src/middlewares/canAccessResource.ts (1)
  • canAccessResourceMiddleware (25-69)
packages/federation-sdk/src/index.ts (1)
  • federationSDK (325-325)
packages/homeserver/src/middlewares/isAuthenticated.ts (1)
packages/federation-sdk/src/index.ts (1)
  • federationSDK (325-325)
packages/homeserver/src/controllers/internal/message.controller.ts (1)
packages/federation-sdk/src/index.ts (1)
  • federationSDK (325-325)
packages/homeserver/src/controllers/federation/invite.controller.ts (2)
packages/homeserver/src/middlewares/isAuthenticated.ts (1)
  • isAuthenticatedMiddleware (4-69)
packages/federation-sdk/src/index.ts (1)
  • federationSDK (325-325)
packages/homeserver/src/controllers/internal/external-federation-request.controller.ts (2)
packages/federation-sdk/src/index.ts (1)
  • federationSDK (325-325)
packages/room/src/manager/event-wrapper.ts (2)
  • event (129-138)
  • roomId (101-103)
packages/homeserver/src/controllers/federation/profiles.controller.ts (3)
packages/homeserver/src/middlewares/isAuthenticated.ts (1)
  • isAuthenticatedMiddleware (4-69)
packages/federation-sdk/src/index.ts (1)
  • federationSDK (325-325)
packages/homeserver/src/middlewares/canAccessResource.ts (1)
  • canAccessResourceMiddleware (25-69)
packages/homeserver/src/controllers/federation/send-join.controller.ts (2)
packages/homeserver/src/middlewares/canAccessResource.ts (1)
  • canAccessResourceMiddleware (25-69)
packages/federation-sdk/src/index.ts (1)
  • federationSDK (325-325)
packages/homeserver/src/controllers/internal/direct-message.controller.ts (1)
packages/federation-sdk/src/index.ts (1)
  • federationSDK (325-325)
packages/homeserver/src/controllers/federation/rooms.controller.ts (2)
packages/homeserver/src/middlewares/isAuthenticated.ts (1)
  • isAuthenticatedMiddleware (4-69)
packages/federation-sdk/src/index.ts (1)
  • federationSDK (325-325)
packages/homeserver/src/controllers/well-known/well-known.controller.ts (1)
packages/federation-sdk/src/index.ts (1)
  • federationSDK (325-325)
packages/homeserver/src/controllers/federation/versions.controller.ts (2)
packages/federation-sdk/src/services/config.service.ts (2)
  • serverName (104-106)
  • version (108-110)
packages/federation-sdk/src/index.ts (1)
  • federationSDK (325-325)
packages/homeserver/src/controllers/federation/transactions.controller.ts (4)
packages/federation-sdk/src/index.ts (1)
  • federationSDK (325-325)
packages/homeserver/src/middlewares/isAuthenticated.ts (1)
  • isAuthenticatedMiddleware (4-69)
packages/federation-sdk/src/services/config.service.ts (1)
  • serverName (104-106)
packages/homeserver/src/middlewares/canAccessResource.ts (1)
  • canAccessResourceMiddleware (25-69)
packages/federation-sdk/src/services/federation-request.service.ts (1)
packages/federation-sdk/src/services/config.service.ts (1)
  • serverName (104-106)
packages/homeserver/src/homeserver.module.ts (1)
packages/federation-sdk/src/index.ts (2)
  • init (264-323)
  • federationSDK (325-325)
packages/homeserver/src/controllers/key/server.controller.ts (1)
packages/federation-sdk/src/index.ts (1)
  • federationSDK (325-325)
packages/homeserver/src/controllers/internal/room.controller.ts (2)
packages/federation-sdk/src/index.ts (3)
  • federationSDK (325-325)
  • UserID (35-35)
  • EventID (34-34)
packages/room/src/types/_common.ts (2)
  • UserID (20-20)
  • EventID (8-8)
packages/homeserver/src/middlewares/canAccessResource.ts (2)
packages/homeserver/src/middlewares/isAuthenticated.ts (1)
  • isAuthenticatedMiddleware (4-69)
packages/federation-sdk/src/index.ts (1)
  • federationSDK (325-325)
packages/federation-sdk/src/index.ts (8)
packages/federation-sdk/src/services/database-connection.service.ts (1)
  • DatabaseConnectionService (4-73)
packages/core/src/models/event.model.ts (2)
  • EventStore (26-38)
  • EventStagingStore (40-44)
packages/federation-sdk/src/repositories/key.repository.ts (1)
  • Key (4-9)
packages/federation-sdk/src/repositories/lock.repository.ts (1)
  • Lock (4-8)
packages/federation-sdk/src/repositories/room.repository.ts (1)
  • Room (6-17)
packages/federation-sdk/src/repositories/server.repository.ts (1)
  • Server (4-12)
packages/federation-sdk/src/repositories/upload.repository.ts (1)
  • Upload (4-12)
packages/federation-sdk/src/repositories/state-graph.repository.ts (1)
  • StateGraphStore (12-26)
packages/federation-sdk/src/sdk.ts (5)
packages/federation-sdk/src/services/config.service.ts (2)
  • singleton (81-151)
  • AppConfig (11-38)
packages/federation-sdk/src/services/federation-request.service.ts (1)
  • singleton (30-209)
packages/federation-sdk/src/services/invite.service.ts (1)
  • singleton (35-241)
packages/federation-sdk/src/services/federation.service.ts (1)
  • singleton (24-352)
packages/federation-sdk/src/index.ts (3)
  • PduType (31-31)
  • EventStore (39-39)
  • PduForType (29-29)
🔇 Additional comments (29)
packages/homeserver/src/controllers/internal/message.controller.ts (2)

2-2: LGTM! Clean refactoring to singleton pattern.

The import change from dependency injection to the federationSDK singleton aligns with the broader SDK refactoring objectives and simplifies the controller's dependencies.


23-146: LGTM! Consistent error handling across endpoints.

The updateMessage, sendReaction, and unsetReaction endpoints all properly wrap federationSDK calls in try-catch blocks and return well-structured error responses.

packages/homeserver/src/controllers/key/server.controller.ts (1)

1-1: Initialization sequence verified—refactoring is safe.

The federationSDK singleton is configured in the setup() function before serverKeyPlugin is registered, ensuring the SDK is ready before any route handlers execute. The async initialization pattern prevents early access issues.

packages/homeserver/src/controllers/internal/invite.controller.ts (1)

31-31: LGTM: federationSDK usage is correct.

The migration to federationSDK.getLatestRoomState() and federationSDK.handlePdu() correctly implements the new singleton facade pattern. Error handling appropriately lets exceptions propagate to the Elysia framework.

Also applies to: 63-63

packages/homeserver/src/controllers/well-known/well-known.controller.ts (1)

1-9: Refactor approved—initialization order is correct.

The federationSDK.init() call at line 45 of homeserver.module.ts happens before the app is created and plugins are registered, ensuring federationSDK.getWellKnownHostData() will have valid state when the route is accessed. The singleton pattern and synchronous call are both appropriate.

packages/federation-sdk/src/services/invite.service.ts (1)

173-174: LGTM - Config access pattern updated consistently.

The migration from getInviteConfig() to getConfig('invite') aligns with the standardized configuration API introduced in this PR.

packages/homeserver/src/controllers/internal/room.controller.ts (2)

47-47: LGTM - Consistent migration to federationSDK.

Room creation now properly delegates to the centralized SDK.


86-90: LGTM - All room operations migrated to federationSDK.

All endpoints consistently use the new SDK facade while preserving response shapes and validation logic.

Also applies to: 133-138, 167-167, 176-176, 214-217, 268-272, 343-343, 376-376, 415-420, 437-437, 443-443, 454-458

packages/federation-sdk/src/services/federation-request.service.ts (1)

43-43: LGTM - Config access updated to new API.

The change from property access to getConfig('serverName') aligns with the standardized configuration pattern introduced in this PR.

packages/homeserver/src/controllers/federation/versions.controller.ts (2)

6-7: Config values retrieved at initialization time.

The serverName and version are now retrieved once when the plugin is initialized rather than per-request. This is acceptable since these values are static configuration that don't change at runtime.


14-15: LGTM - Response construction updated consistently.

The response properly uses the pre-fetched config values.

packages/federation-sdk/src/services/federation-request.service.spec.ts (1)

94-103: LGTM - Mock updated to support new config API.

The mock ConfigService now implements the getConfig(key) method, properly returning mockServerName for the 'serverName' key and throwing for unknown keys. The retained serverName property on line 100 may be used elsewhere in tests.

packages/homeserver/src/controllers/internal/direct-message.controller.ts (1)

31-34: LGTM - Direct message creation migrated to federationSDK.

The endpoint now properly delegates to the centralized SDK while maintaining error handling and response structure.

packages/homeserver/src/controllers/federation/media.controller.ts (2)

18-18: LGTM - Middleware simplified.

The isAuthenticatedMiddleware() is now invoked without service arguments, as it resolves dependencies internally via federationSDK.


39-39: LGTM - Access control middleware updated.

The canAccessResourceMiddleware now only requires the resource type, with internal dependency resolution through federationSDK.

packages/federation-sdk/src/services/federation.service.ts (1)

11-11: No circular dependency detected; lazy injection appears unnecessary for this service pair.

Verification found that StateService has no dependency on FederationService. The StateService constructor depends only on stateRepository, eventRepository, and configService. Since no circular dependency exists between FederationService and StateService, the delay() pattern used for lazy injection of StateService is not addressing an actual circular dependency issue.

The lazy injection is not harmful, but the premise that it resolves a circular dependency between these two services is not supported by the codebase analysis.

Likely an incorrect or invalid review comment.

packages/homeserver/src/middlewares/isAuthenticated.ts (2)

22-38: Body parameter addition improves signature verification.

The addition of the body parameter to verifyRequestSignature (line 37) ensures that request signatures are verified against the complete request including the body. This is a security improvement that helps prevent tampering with request payloads.

The body extraction logic correctly handles cases where the body might not be parseable (lines 28-30).


1-4: The initialization order concern is not valid based on code structure.

The middleware is registered as part of the setup() function which is async and calls await init() before creating the Elysia app and registering any controllers or plugins. This ensures that federationSDK is fully initialized before isAuthenticatedMiddleware is registered or executed.

The flow is:

  1. setup() awaits init({...})
  2. setup() calls federationSDK.setConfig({...})
  3. setup() creates the app and registers all plugins/controllers (which use isAuthenticatedMiddleware())
  4. Consumers must await appPromise or call setup() before starting the server

The middleware's call to federationSDK.verifyRequestSignature() happens at request time within an async handler, long after initialization has completed.

Likely an incorrect or invalid review comment.

packages/federation-sdk/src/services/state.service.spec.ts (1)

120-133: Test correctly updated to reflect DatabaseConnectionService constructor change.

The test instantiation of DatabaseConnectionService now passes a plain configuration object instead of a ConfigService instance, which aligns with the updated constructor signature in the production code.

packages/homeserver/src/controllers/federation/state.controller.ts (1)

2-2: LGTM! Consistent migration to federationSDK facade.

The controller has been successfully migrated to use the federationSDK singleton:

  • Service method calls replaced with federationSDK.getStateIds() and federationSDK.getState()
  • Middleware usage simplified to pass only the entity type
  • Request/response handling and DTOs remain unchanged

Also applies to: 17-24, 42-45

packages/homeserver/src/middlewares/canAccessResource.ts (2)

1-1: Verify federationSDK initialization before middleware usage.

Similar to isAuthenticatedMiddleware, this middleware now depends on the global federationSDK singleton. Ensure initialization order is correct to prevent runtime errors.

Also applies to: 25-29


49-53: LGTM! Access control logic preserved.

The resource access check has been successfully migrated to federationSDK.canAccessResource() while maintaining the same authorization logic and error handling.

packages/federation-sdk/src/services/database-connection.service.ts (1)

41-41: LGTM! Config access updated correctly.

The config access has been correctly updated to use this.config directly instead of calling this.configService.getDatabaseConfig().

packages/homeserver/src/controllers/federation/send-join.controller.ts (1)

2-2: LGTM! Successfully migrated to federationSDK.

The controller has been correctly updated to use federationSDK.sendJoin(). The query parameter destructuring (line 17) is preserved per the comment.

Also applies to: 12-25

packages/homeserver/src/controllers/internal/external-federation-request.controller.ts (4)

10-10: LGTM! Request signing migrated to federationSDK.

The signed request functionality has been successfully migrated to use federationSDK.makeSignedRequest() while preserving all request parameters.

Also applies to: 18-24


71-85: LGTM! Event building consistently migrated.

All event building operations (m.room.member, m.room.message, m.room.power_levels, m.room.join_rules, m.room.topic, m.room.name) have been successfully migrated to use federationSDK.buildEvent(), maintaining consistent patterns across all event types.

Also applies to: 88-101, 120-134, 137-151, 154-168, 171-185


113-119: LGTM! State retrieval migrated correctly.

The room state retrieval has been updated to use federationSDK.getLatestRoomState2() with proper error handling for non-existent state.


223-224: LGTM! PDU handling migrated to federationSDK.

Both PDU processing (handlePdu) and event distribution (sendEventToAllServersInRoom) have been correctly migrated to use the federationSDK facade.

packages/homeserver/src/controllers/federation/rooms.controller.ts (1)

1-1: LGTM! Public rooms endpoints migrated successfully.

Both GET and POST endpoints for public rooms have been successfully updated to use federationSDK.getAllPublicRoomIdsAndNames(). The response augmentation with defaultObj and filter logic remain unchanged, ensuring backward compatibility.

Also applies to: 7-7, 20-20, 64-64

container.resolve(StagingAreaListener);
}

export const federationSDK = container.resolve(FederationSDK);
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Delay federationSDK resolution until after init.

Resolving FederationSDK at module load happens before init() registers the Mongo-backed tokens (e.g., RoomCollection). Tsyringe then fails with ResolutionError: Attempted to resolve unregistered dependency token "RoomCollection" as soon as any consumer imports { federationSDK }, preventing the app from ever calling init(). Please defer the resolution until after init completes—store the instance during init or expose a lazy getter that resolves once the container setup is done. For example:

-export const federationSDK = container.resolve(FederationSDK);
+let federationSDKInstance: FederationSDK | null = null;
+
+export function getFederationSDK(): FederationSDK {
+	if (!federationSDKInstance) {
+		throw new Error('Call init() before accessing federationSDK');
+	}
+	return federationSDKInstance;
+}

and set federationSDKInstance = container.resolve(FederationSDK); at the end of init().

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In packages/federation-sdk/src/index.ts around line 325, the module currently
resolves FederationSDK at import time which triggers a ResolutionError because
init() hasn't registered required tokens like RoomCollection; change this so the
container.resolve(FederationSDK) is not called at module load: remove the
immediate export and instead provide a variable (e.g., federationSDKInstance)
that is set inside init() after all tokens are registered or replace the
exported value with a lazy getter function that calls
container.resolve(FederationSDK) on first access; ensure init() assigns the
instance at the end and exports either the instance or the getter so consumers
get a resolved SDK only after init completes.

Comment on lines +39 to +52
export async function setup() {
const envPath = path.resolve(process.cwd(), '.env');
if (fs.existsSync(envPath)) {
dotenv.config({ path: envPath });
}

const config = new ConfigService({
instanceId: crypto.randomUUID(),
serverName: process.env.SERVER_NAME || 'rc1',
port: Number.parseInt(process.env.SERVER_PORT || '8080', 10),
database: {
await init({
dbConfig: {
uri: process.env.MONGODB_URI || 'mongodb://localhost:27017/matrix',
name: process.env.DATABASE_NAME || 'matrix',
poolSize: Number.parseInt(process.env.DATABASE_POOL_SIZE || '10', 10),
},
});

Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Don’t drop the emitter injection path

setup no longer accepts options, so we can’t pass an emitter down to init. That forces the SDK to spin up its standalone emitter and breaks deployments that previously wired the homeserver into the existing event bus. Please keep the optional parameter and forward it to init.

-export async function setup() {
+export async function setup(options?: HomeserverSetupOptions) {-	await init({
-		dbConfig: {
+	await init({
+		emitter: options?.emitter,
+		dbConfig: {
 			uri: process.env.MONGODB_URI || 'mongodb://localhost:27017/matrix',
 			name: process.env.DATABASE_NAME || 'matrix',
 			poolSize: Number.parseInt(process.env.DATABASE_POOL_SIZE || '10', 10),
 		},
 	});

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In packages/homeserver/src/homeserver.module.ts around lines 39 to 52, the setup
function lost its optional emitter parameter and thus cannot forward a provided
emitter to init; restore the optional parameter (e.g., setup(emitter?:
EventEmitterType) or matching SDK type) in the function signature, pass that
emitter through to the init call (e.g., init({ ..., emitter })) while preserving
existing defaults and types, and update any callers if needed so existing
deployments can continue to inject their emitter.

ggazzo
ggazzo previously approved these changes Nov 3, 2025
@ggazzo ggazzo merged commit 7a2ad55 into main Nov 3, 2025
3 checks passed
@ggazzo ggazzo deleted the feat-create-sdk-class branch November 3, 2025 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants