refactor: improve type safety in invite and state services

[ggazzo]

Summary by CodeRabbit

• Refactor
  • Enhanced type safety for event handling and federation domain processing
  • Improved error handling with clearer type definitions for authorization rules
  • Optimized domain extraction logic for federation invites

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

This PR enhances type safety and adds domain extraction capabilities to PersistentEventBase by introducing generic type parameterization, a new PduTypeWithoutStateKey type, refined State interface with narrowed Map methods, and a new stateKeyDomain getter property. The invite service is updated to use the new stateKeyDomain directly instead of parsing state_key manually.

Changes

Cohort / File(s)	Summary
Core Type Refinements packages/room/src/manager/event-wrapper.ts	Introduces PduTypeWithoutStateKey type; refactors State interface with narrowed Map and overloaded get method; makes rawEvent field generic via PduForType<Type> parameter; enhances stateKey with conditional typing that returns undefined for PDU types without state_key; adds new stateKeyDomain getter with conditional return type and runtime domain extraction logic.
PersistentEventBase Usage Updates packages/federation-sdk/src/services/state.service.ts, packages/room/src/authorizartion-rules/errors.ts	Updates method signatures and constructor parameters to use PersistentEventBase<any, any> instead of bare PersistentEventBase for proper generic type compliance across addAuthEvents, addPrevEvents, signEvent, and StateResolverAuthorizationError.
Invite Domain Extraction packages/federation-sdk/src/services/invite.service.ts	Replaces domain extraction logic from full state_key parsing to direct stateKeyDomain property access; removes validation error path for missing state_key; adjusts conditional to use stateKeyDomain === serverName for local-server invite handling.
Authorization Logic Refactoring packages/room/src/authorizartion-rules/rules.ts	Refactors membership change validation in isMembershipChangeAllowed to use explicit getContent() call with type cast instead of direct content property access; maintains existing behavior with improved type safety.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

• feat: add room invites support #300 — Directly depends on the new stateKeyDomain property introduced in event-wrapper.ts for invite handling domain extraction.
• feat: partial states (basic) #267 — Modifies the same core PersistentEventBase and StateService types, requiring coordinated type parameter updates.
• refactor: create State type to replace Map #311 — Modifies the State interface in event-wrapper.ts with similar Map narrowing and interface refinements.

Suggested reviewers

• sampaiodiego
• rodrigok

Poem

🐰 Types now flourish with generics so true,
StateKeyDomain arrives—a domain for you!
Invite logic flows cleaner, no parsing to chase,
PersistentEventBase finds its proper place. ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately reflects the primary changes in the PR, which involve adding generic type parameters to PersistentEventBase, improving type safety across invite and state services.

_{✏️ 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
• Commit unit tests in branch chore/refactor-statekey

---

🧹 Recent nitpick comments

packages/room/src/manager/event-wrapper.ts (1)

157-164: Consider documenting the throwing behavior.

The stateKeyDomain getter throws when stateKey is undefined. While the return type never for PduTypeWithoutStateKey signals this at the type level, callers working with PersistentEventBase (without specific type parameters) won't get compile-time protection. Consider adding a JSDoc comment to clarify this behavior.

📝 Suggested documentation

+	/**
+	 * Extracts the domain from the state_key.
+	 * `@throws` {Error} If stateKey is undefined (e.g., for non-state events).
+	 */
 	get stateKeyDomain(): Type extends PduTypeWithoutStateKey ? never : string {

---

📜 Recent review details

Configuration used: Organization 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 252f452 and a042db4.

📒 Files selected for processing (5)

• packages/federation-sdk/src/services/invite.service.ts
• packages/federation-sdk/src/services/state.service.ts
• packages/room/src/authorizartion-rules/errors.ts
• packages/room/src/authorizartion-rules/rules.ts
• packages/room/src/manager/event-wrapper.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/room/src/manager/event-wrapper.ts

🧬 Code graph analysis (4)

packages/room/src/authorizartion-rules/errors.ts (1)

packages/federation-sdk/src/index.ts (1)

• PersistentEventBase (30-30)

packages/room/src/authorizartion-rules/rules.ts (1)

packages/room/src/manager/event-wrapper.ts (1)

• event (172-181)

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

packages/federation-sdk/src/index.ts (1)

• PersistentEventBase (30-30)

packages/room/src/manager/event-wrapper.ts (5)

packages/federation-sdk/src/index.ts (6)

• PduType (29-29)
• PduForType (27-27)
• State (50-50)
• PersistentEventBase (30-30)
• RoomVersion (31-31)
• extractDomainFromId (143-143)

packages/room/src/types/v3-11.ts (1)

• PduType (51-51)

packages/room/src/types/_common.ts (2)

• PduForType (26-26)
• StateMapKey (22-22)

packages/federation-sdk/src/specs/federation-api.ts (1)

• State (171-171)

packages/room/src/manager/type.ts (1)

• RoomVersion (14-14)

⏰ 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: cubic · AI code reviewer

🔇 Additional comments (10)

packages/room/src/authorizartion-rules/rules.ts (1)

177-182: LGTM!

The refactored code safely accesses the creator property through getContent() with an explicit type cast. This aligns with the PR's goal of improving type safety while maintaining the same runtime behavior.

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

98-98: LGTM!

Using stateKeyDomain directly is cleaner and centralizes the domain extraction logic. The getter appropriately throws if stateKey is undefined or malformed, which aligns with the validation that was previously done manually in this file.

packages/room/src/authorizartion-rules/errors.ts (1)

26-28: LGTM!

Widening the type parameters to PersistentEventBase<any, any> is appropriate here since the error class only needs the base methods (toStrippedJson(), eventId) which are available regardless of the specific type parameters.

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

297-297: LGTM!

The type widening to PersistentEventBase<any, any> is appropriate since addAuthEvents only uses base class methods (roomId, getAuthEventStateKeys(), authedBy()).

---

310-310: LGTM!

Same rationale applies here - addPrevEvents uses only base class functionality.

---

333-333: LGTM!

The generic constraint widening maintains the method's ability to return the same type T while accepting any PersistentEventBase variant.

packages/room/src/manager/event-wrapper.ts (4)

45-48: LGTM!

Well-designed utility type that correctly filters PduType to only those types whose corresponding PduForType doesn't include a state_key property.

---

51-61: LGTM!

The refined State interface with the overloaded get method provides better type inference. When accessing state by a key like "m.room.member:@user:domain", TypeScript can now infer the specific PersistentEventBase<RoomVersion, 'm.room.member'> return type.

---

76-76: LGTM!

Making rawEvent generic with PduForType<Type> improves type safety by ensuring the raw event structure matches the declared event type parameter.

---

143-155: LGTM!

The conditional return type accurately reflects the runtime behavior - returning undefined for event types without state_key and the actual state_key value for state events. The type assertion is necessary given the complexity of the conditional type.

_{✏️ 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.}

[codecov-commenter]

Codecov Report

❌ Patch coverage is 44.44444% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 51.97%. Comparing base (252f452) to head (a042db4).

Files with missing lines	Patch %	Lines
packages/room/src/manager/event-wrapper.ts	22.22%	14 Missing ⚠️
packages/room/src/authorizartion-rules/rules.ts	80.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #327      +/-   ##
==========================================
- Coverage   52.29%   51.97%   -0.33%     
==========================================
  Files          97       97              
  Lines       13153    13161       +8     
==========================================
- Hits         6878     6840      -38     
- Misses       6275     6321      +46     

☔ 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.

[cubic-dev-ai]

No issues found across 5 files