Skip to content

regression(native-federation): fix typing loop between instances #37115

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ggazzo merged 1 commit into from
Oct 1, 2025
Merged

regression(native-federation): fix typing loop between instances #37115

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/meteor/server/modules/listeners/listeners.module.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ export class ListenersModule {
});

service.onEvent('user.activity', ({ isTyping, roomId, user }) => {
notifications.notifyRoom(roomId, 'user-activity', user, isTyping ? ['user-typing'] : []);
notifications.notifyRoomInThisInstance(roomId, 'user-activity', user, isTyping ? ['user-typing'] : []);
});

service.onEvent('watch.messages', async ({ message }) => {
Expand Down
8 changes: 1 addition & 7 deletions ee/packages/federation-matrix/src/events/edu.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,8 @@ export const edus = async (emitter: Emitter<HomeserverEventSignatures>, eduProce
return;
}

const matrixUser = await Users.findOne({ 'federation.mui': data.user_id });
Copy link
Member Author

@sampaiodiego sampaiodiego Oct 1, 2025

Choose a reason for hiding this comment

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

this find was useless since data.user_id is the same as matrixUser.username .. we also don't need to check if it exists since this comes from homeserver which already has done validations

if (!matrixUser?.username) {
logger.debug(`No bridged user found for Matrix user_id: ${data.user_id}`);
return;
}

void api.broadcast('user.activity', {
user: matrixUser.username,
user: data.user_id,
isTyping: data.typing,
roomId: matrixRoom._id,
});
Comment on lines 23 to 27
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 1, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify downstream consumers and local user filtering.

The typing handler now uses data.user_id directly without validating that the user exists or is federated, unlike the presence handler (lines 39-48) which performs these checks. This raises two concerns:

  1. Inconsistent validation pattern: The presence handler validates the user is federated before processing, but typing does not. If Matrix sends typing events for local users back to the local instance, this could cause the self-typing issue described in the PR objectives.

  2. Downstream consumer compatibility: The previous code used matrixUser.username (a local username), now it uses data.user_id (a Matrix user ID). Verify that consumers of the user.activity broadcast can handle Matrix user ID format.

Run the following script to check how user.activity events are consumed:

Additionally, clarify:

  • Does the Matrix homeserver send typing events for local users back to the local instance, or only for remote users?
  • Is the filtering of local users handled by the notifyRoomInThisInstance change mentioned in the AI summary?

🏁 Script executed:

#!/bin/bash
# Find all consumers of 'user.activity' broadcast events
rg -n -C5 "on\('user\.activity'" --type=ts --type=js

# Also search for any user.activity string references
rg -n -C3 "'user\.activity'|\"user\.activity\"" --type=ts --type=js

Length of output: 1732

🏁 Script executed:

#!/bin/bash
set -e
# Show context around the onEvent handler in listeners.module.ts
sed -n '150,230p' apps/meteor/server/modules/listeners/listeners.module.ts

# Find import of notifications
grep -R "import .*notifications" -n apps/meteor/server/modules/listeners/listeners.module.ts

# Search for definition of notifyRoomInThisInstance
rg -n "notifyRoomInThisInstance" -C5 apps/meteor/server/modules

Length of output: 9073

Restore user validation and map data.user_id to a local user ID before broadcasting (ee/packages/federation-matrix/src/events/edu.ts L23-27)
The presence handler still looks up and validates the federated user before emitting, but the typing handler now sends the raw Matrix user ID—downstream consumers rely on internal user IDs to filter out self-typing and drive UI logic correctly.

🤖 Prompt for AI Agents 
In ee/packages/federation-matrix/src/events/edu.ts around lines 23–27, the
typing handler currently broadcasts the raw Matrix user ID; restore the
federated-user lookup/validation used in the presence handler and map
data.user_id to the local user record before broadcasting. Specifically,
fetch/validate the local user (using the same helper used by presence, e.g. the
federated user lookup), bail out if the federated user is not found/invalid, and
pass user: localUser._id (not data.user_id) in the api.broadcast payload; keep
existing error handling/logging when lookup fails.

Expand Down
Loading