fix: use invite_room_state from payload #37249
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 |
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughUpdated the federation-sdk dependency from 0.1.31 to 0.1.32 across two package.json files. Enhanced the Matrix invite API to capture and validate room state events (invite_room_state), requiring an m.room.create event, and added logging for error handling in the invite processing flow. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Invite Endpoint
participant Validation
participant Processing
Client->>Invite Endpoint: POST /v2/invite/:roomId/:eventId<br/>(with invite_room_state)
Invite Endpoint->>Validation: Extract strippedStateEvents
Validation->>Validation: Check for m.room.create event
alt m.room.create exists
Validation-->>Invite Endpoint: ✓ Valid
Invite Endpoint->>Processing: invite.processInvite(strippedStateEvents)
Processing->>Processing: startJoiningRoom(strippedStateEvents)
Processing-->>Invite Endpoint: Success
Invite Endpoint-->>Client: 200 OK
else m.room.create missing
Validation-->>Invite Endpoint: ✗ Invalid
Invite Endpoint->>Invite Endpoint: logger.error()
Invite Endpoint-->>Client: 400 M_MISSING_PARAM
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes The changes consist of straightforward dependency bumps (trivial) and focused validation/logging additions to a single API endpoint with consistent patterns—validation, data threading, and error handling that follow established conventions. Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 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 (3)
⏰ 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 (6)
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 |
sampaiodiego
force-pushed
the
fix-invite_room_state-validation
branch
from
f63d6f1 to
6a1c8d9
Compare
sampaiodiego
force-pushed
the
fix-invite_room_state-validation
branch
from
19b42e4 to
358ea34
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR fixes Matrix federation invite handling by properly using the invite_room_state from the payload and adding validation to ensure required room state events are present.
- Updates federation SDK dependency from 0.1.31 to 0.1.32
- Adds validation for required
m.room.createevent in invite room state - Passes stripped state events to the invite processing function
Reviewed Changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/core-services/package.json | Updates federation SDK dependency to 0.1.32 |
| ee/packages/federation-matrix/package.json | Updates federation SDK dependency to 0.1.32 |
| ee/packages/federation-matrix/src/api/_matrix/invite.ts | Adds invite room state validation and error logging |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| throw new Error('join event has missing state key, unable to determine user to join'); | ||
| } | ||
|
|
||
| if (!strippedStateEvents?.some((e: any) => e.type === 'm.room.create')) { |
There was a problem hiding this comment.
Using any type weakens type safety. Consider defining a proper interface for the state event or using a more specific type that includes at least the type property.
| if (!strippedStateEvents?.some((e: any) => e.type === 'm.room.create')) { | |
| if (!strippedStateEvents?.some((e: { type: string }) => e.type === 'm.room.create')) { |
| }; | ||
| } | ||
|
|
||
| logger.error({ msg: 'Error processing invite', err: error }); |
There was a problem hiding this comment.
The error logging occurs after the response is returned, which means the logging won't happen if the function returns early. Consider moving this before the return statement or using a more structured error handling approach.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## release-7.11.0 #37249 +/- ##
=================================================
Coverage ? 66.35%
=================================================
Files ? 3386
Lines ? 115636
Branches ? 21355
=================================================
Hits ? 76729
Misses ? 36298
Partials ? 2609
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Further comments
Summary by CodeRabbit
Bug Fixes
Chores