Skip to content

Conversation

@rodrigok
Copy link
Member

@rodrigok rodrigok commented Sep 25, 2025

Summary by CodeRabbit

  • Bug Fixes
    • Improved reliability of membership notifications when users join or accept invites.
    • User avatars now display more accurately on join events, reducing cases where an avatar might appear missing.
    • Preserved existing display name behavior to avoid unexpected changes in member lists and notifications.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 25, 2025

Walkthrough

Updated the send-join service to emit membership events on a new channel, include a state_key set to the sender, and source avatar_url directly from the signed event content. Other content fields remain unchanged.

Changes

Cohort / File(s) Summary
Membership event emission update
packages/federation-sdk/src/services/send-join.service.ts
Changed emit channel from homeserver.matrix.accept-invite to homeserver.matrix.membership; added state_key using sender; avatar_url now taken directly from signedJoinEvent.getContent().avatar_url (removed null fallback); kept displayname fallback to empty string and existing membership handling.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant C as Client
  participant S as SendJoinService
  participant E as EventBus

  Note over S: Build signed join event
  C->>S: requestSendJoin()
  S->>S: extract content (displayname, avatar_url)
  S->>S: set state_key = sender
  S->>E: emit "homeserver.matrix.membership"<br/>payload: { state_key, content.avatar_url, ... }
  E-->>S: ack
  S-->>C: result
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I thump my paws—new channels ring,
A state_key hops in on springy string.
Avatar shines, no fallback night,
Membership drums to a fresher rite.
In burrows of code where packets flow,
I nibble bytes and watch it go. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Title Check ⚠️ Warning The title suggests adding support for third-party invites, but the changes instead focus on updating the send-join service to emit membership events with a new state_key field and direct avatar_url from the join event content. There is no explicit implementation of third-party invite handling in the diff, making the title misleading. The main modifications are the event channel adjustment and payload field updates, which the title does not convey. Update the title to clearly reflect the changes, for example: "feat: update send-join service to emit membership events with state_key and direct avatar_url" so it accurately summarizes the main modifications.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/3rd-party-invite

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 Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.67%. Comparing base (5fddd48) to head (aeb47ea).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #228   +/-   ##
=======================================
  Coverage   81.67%   81.67%           
=======================================
  Files          63       63           
  Lines        4682     4682           
=======================================
  Hits         3824     3824           
  Misses        858      858           

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

@ggazzo ggazzo merged commit a16509d into main Sep 25, 2025
2 of 3 checks passed
@ggazzo ggazzo deleted the feat/3rd-party-invite branch September 25, 2025 21:48
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: 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 5fddd48 and aeb47ea.

📒 Files selected for processing (1)
  • packages/federation-sdk/src/services/send-join.service.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/federation-sdk/src/services/send-join.service.ts (2)
packages/room/src/manager/v3.ts (1)
  • eventId (22-34)
packages/room/src/manager/event-wrapper.ts (1)
  • roomId (75-77)

Comment on lines +74 to 79
this.emitterService.emit('homeserver.matrix.membership', {
event_id: eventId,
room_id: roomId,
sender: signedJoinEvent.sender,
state_key: signedJoinEvent.sender,
origin_server_ts: signedJoinEvent.originServerTs,
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

Restore state_key to target user instead of sender

For m.room.member events the state_key must always be the user whose membership state is changing. Invites, kicks, bans, and third‑party invites all have sender !== state_key. By copying the sender here we’ll publish the wrong membership target, so downstream consumers will update the inviter rather than the invitee—precisely what this PR is trying to fix. Please pull the actual state key from the event payload instead.

-			state_key: signedJoinEvent.sender,
+			state_key: signedJoinEvent.event.state_key,
🤖 Prompt for AI Agents
packages/federation-sdk/src/services/send-join.service.ts around lines 74 to 79:
the emitted m.room.member payload sets state_key to the sender, which is
incorrect for membership events; change the code to read the actual state key
from the signed event payload (use the event's state_key property — e.g.
signedJoinEvent.state_key or signedJoinEvent.stateKey depending on naming) and
emit that as state_key so the membership target (invitee/kicked/banned user) is
published correctly.

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