Skip to content

fix(chat): show typing only from participants of the current conversation - #6905

Open
ToteMeiSter wants to merge 1 commit into
nextcloud:masterfrom
ToteMeiSter:fix/typing-indicator-room-sessions-v3
Open

ToteMeiSter wants to merge 1 commit into
nextcloud:masterfrom
ToteMeiSter:fix/typing-indicator-room-sessions-v3

Conversation

@ToteMeiSter

@ToteMeiSter ToteMeiSter commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Problem

A conversation shows "Guest is typing…". The person who types is not a guest. They type in another conversation.

Cause

  • The signaling connection of an account is shared by all conversations. The session map in WebSocketInstance (usersHashMap) was filled by join events and cleared only by leave events. It was not cleared when the room changed, when the room was left, after a hello without resume, or on no_such_session / no_such_room. So it kept the sessions of earlier rooms.
  • MessageInputFragment sent startedTyping / stoppedTyping to every session in the map.
  • The signaling server (HPB) delivers a message with recipient session to that session in any room. In nextcloud-spreed-signaling, server/hub.go processMessageMsg checks only "same backend" and "not to self". It does not check the room of the recipient.
  • The receiver did not check that the sender is in the current room. The sender session was unknown, so its name was empty, and the app showed "Guest".
  • The web client already filters: src/utils/SignalingTypingHandler.js in spreed sends typing only to the participants of the current room and ignores typing from sessions which are not in that list.

Changes

  1. Track only the signaling sessions of the current room.
    • New RoomSessionRegistry replaces usersHashMap. It is thread safe. It is cleared when the server answers with another room, when the room is left, after a hello without resume, and on no_such_session / no_such_room. A resumed connection keeps it.
    • WebSocketInstance gives a snapshot of the room sessions without the own session, and checks if a session is in the current room.
    • MessageInputFragment sends typing only to the sessions of the current room.
    • ChatActivity accepts typing only from a session of the current room. A real guest still joins without a user id, stays in the registry and is still shown as "Guest".
  2. Drop late join events of a left room. After a leave the room is unknown, so the answer to the leave (an empty room) also clears the sessions. Join events of the left room which arrive between the leave and its answer are dropped.
  3. Hide the typing indicator of a session which left the room.
    • SignalingMessageReceiver and ConversationMessageNotifier pass the leave event to the chat listener (new onSessionsLeft, empty by default).
    • TypingParticipant stores the session and can remove the participants of left sessions.
    • ChatActivity changes the typing participants on the UI thread only, also for a stop message.
    • Without this, the indicator of a participant who left stayed until the 15 second timeout.
  4. Stop and expire typing per session, not per participant.
    • Every session of a typing user has its own 15 second timer. stoppedTyping and the timeout forget only that session; the participant is removed with its last session.
    • Before, a stop of one device hid the indicator while another device of the same user still typed.

Tests

  • RoomSessionRegistryTest
  • TypingParticipantTest
  • SignalingMessageReceiverSessionsLeftTest

How to test

  1. Use two accounts, X and Y, on a server with the high-performance backend. Conversation A has X and Y. Conversation B has Y and another person, not X.
  2. X and Y open A at the same time.
  3. Y opens B.
  4. X opens another conversation and starts to type there.
  5. Before: Y sees "Guest is typing…" in B. After: Y sees no typing indicator in B.
  6. A real guest who joins by a public link and types is still shown as "Guest".

Tested on a Huawei DEL-LX9 and a Samsung SM-S926B (Android 17): "Guest is typing…" from another conversation no longer appears.

🖼️ Screenshots

🏚️ Before 🏡 After
"Guest is typing…" from another conversation No typing indicator

No screenshots. The UI change is the absence of a wrong typing line.

🚧 TODO

  • Nothing left

🏁 Checklist

  • ⛑️ Tests (unit and/or integration) are included or not needed
  • 🔖 Capability is checked or not needed (not needed: client side only, no new server feature)
  • 🔙 Backport requests are created or not needed: /backport to stable-xx.x
  • 📅 Milestone is set
  • 🌸 PR title is meaningful (if it should be in the changelog: is it meaningful to users?)

🤖 AI (if applicable)

  • The content of this PR was partly or fully generated using AI

The code and the tests were written by an AI agent (Claude Code, model claude-sonnet-5-5). I tested the change on a device and I am responsible for it. The commits carry the trailer Assisted-by: Claude-Code:claude-sonnet-5-5. This description was also prepared with AI help and proofread by me.

@coderabbitai

coderabbitai Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: f4aa4ddd-1f82-4ecf-b619-7bc06b18b582
📥 Commits

Reviewing files that changed from the base of the PR and between 1211e20 and c7d2f34.

📒 Files selected for processing: 1
  • app/src/test/java/com/nextcloud/talk/webrtc/RoomSessionRegistryTest.kt
🚧 Files skipped from review as they are similar to previous changes: 1
  • app/src/test/java/com/nextcloud/talk/webrtc/RoomSessionRegistryTest.kt

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.

📝 Walkthrough

RoomSessionRegistry tracks sessions for the current room and provides session lookups. WebSocketInstance updates or resets that registry as room state changes. Signaling forwards room departure session IDs to conversation listeners. Chat typing behavior uses current-room sessions for typing messages and removes typing participants when their sessions leave.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to c7d2f

No actionable merge-blocking issue is established for this change; it is mergeable after normal checks.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 16.22% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 74 functions across 10 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: restricting typing indicators to participants in the current conversation.
Description check Passed The description is complete and follows the repository template. It explains the problem, cause, changes, tests, manual verification, TODO status, and checklist. The backport and milestone items remai…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description is complete and follows the repository template. It explains the problem, cause, changes, tests, manual verification, TODO status, and checklist. The backport and milestone items remain unchecked, but these are non-critical metadata items.

✨ Finishing Touches: 💡 1
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 1c7b5586-699b-423c-b95c-264054a80196
📥 Commits

Reviewing files that changed from the base of the PR and between da0a3d9 and f199745.

📒 Files selected for processing: 10
  • app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt
  • app/src/main/java/com/nextcloud/talk/chat/MessageInputFragment.kt
  • app/src/main/java/com/nextcloud/talk/chat/TypingParticipant.kt
  • app/src/main/java/com/nextcloud/talk/signaling/ConversationMessageNotifier.kt
  • app/src/main/java/com/nextcloud/talk/signaling/SignalingMessageReceiver.kt
  • app/src/main/java/com/nextcloud/talk/webrtc/RoomSessionRegistry.kt
  • app/src/main/java/com/nextcloud/talk/webrtc/WebSocketInstance.kt
  • app/src/test/java/com/nextcloud/talk/chat/TypingParticipantTest.kt
  • app/src/test/java/com/nextcloud/talk/signaling/SignalingMessageReceiverSessionsLeftTest.kt
  • app/src/test/java/com/nextcloud/talk/webrtc/RoomSessionRegistryTest.kt

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

Comment thread app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt
Comment thread app/src/main/java/com/nextcloud/talk/webrtc/RoomSessionRegistry.kt

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

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 info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: da5969f1-13d9-4629-8c8f-b3b1801fc10f
📥 Commits

Reviewing files that changed from the base of the PR and between f199745 and 1517c84.

📒 Files selected for processing: 5
  • app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt
  • app/src/main/java/com/nextcloud/talk/chat/TypingParticipant.kt
  • app/src/main/java/com/nextcloud/talk/webrtc/RoomSessionRegistry.kt
  • app/src/test/java/com/nextcloud/talk/chat/TypingParticipantTest.kt
  • app/src/test/java/com/nextcloud/talk/webrtc/RoomSessionRegistryTest.kt

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

Comment thread app/src/main/java/com/nextcloud/talk/webrtc/RoomSessionRegistry.kt
@ToteMeiSter

Copy link
Copy Markdown
Contributor Author

I added one more commit: 1517c84 "fix(chat): stop and expire typing per session, not per participant".

de29297 keeps all sessions of a typing user, but only the leave event used them. A stoppedTyping of one session still removed the whole participant, so the indicator vanished while the other device of the same user still typed. The single timer of the participant had a similar effect: it was restarted by any session, so after one session stopped, the other one stayed visible up to 15 s longer than its own last startedTyping.

Now every session has its own 15 s timer, and stoppedTyping forgets only the sending session. The participant is removed with its last session. Typing events without a session id are ignored, because isSessionInCurrentRoom(null) is always false and they were never shown anyway.

Tests: TypingParticipantTest covers stop of one of two sessions, expiry of each session on its own, and a repeated start of the same session.

How to test: log in with the same user on two devices, open the conversation on a third device. Type on both devices, then stop typing on one of them. "… is typing" stays visible. Stop on the second device: the indicator disappears.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

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 info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 2320d593-5685-463c-9573-ca56421955b7
📥 Commits

Reviewing files that changed from the base of the PR and between 1517c84 and b403c07.

📒 Files selected for processing: 2
  • app/src/main/java/com/nextcloud/talk/webrtc/RoomSessionRegistry.kt
  • app/src/test/java/com/nextcloud/talk/webrtc/RoomSessionRegistryTest.kt

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.

Comment thread app/src/test/java/com/nextcloud/talk/webrtc/RoomSessionRegistryTest.kt Outdated

@rapterjet2004 rapterjet2004 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, the room scoping looks right: clearing the registry on a room change before currentRoomToken is updated, ignoring joins while no room is known, the per-session typing timers and keeping the typing map on the main thread all look good.

Change requested: the new test doesn't compile against master. WebSocketInstanceRoomSessionsTest.kt imports okhttp3.mockwebserver.*, but master replaced that dependency with mockwebserver3. CI tests the merge ref, so compileGplayDebugUnitTestKotlin fails with "Unresolved reference 'mockwebserver'" and no unit tests run at all. A rebase and a port to the mockwebserver3 API (as in #6828) should fix it.

Non-blocking:

  • The test seams add three test-only constructor parameters (testOkHttpClient, testEventBus, testConnectionHelper, @JvmOverloads) plus an init branch that skips injection to WebSocketInstance, and a test-only constructor to WebSocketConnectionHelper. Behaviour is unchanged with the null defaults, but it would be nicer to keep test scaffolding out of the production classes if there's a reasonable way.
  • Typing is now only accepted from sessions in the registry. Could you confirm with one manual test that typing from a federated participant still shows? I couldn't verify that federated sessions arrive as join events on this websocket.
  • Typing that arrives between joining a room and receiving its join list is dropped. That's fine, the next keystroke re-sends it.

…tion

The signaling connection of an account is shared by all conversations, but
the session map of WebSocketInstance was filled by join events and cleared
only by leave events. It kept the sessions of earlier rooms, so typing
messages went to sessions of other rooms and typing from such sessions was
shown as "Guest".

- RoomSessionRegistry replaces the session map. It holds only the sessions
  of the current room and is cleared when the server answers with another
  room, when the room is left, after a hello without resume and on
  no_such_session / no_such_room. A resumed connection keeps it.
- Join events of a left room are dropped: after a leave the room is unknown,
  so the answer to the leave clears the sessions as well.
- Typing messages are sent only to the sessions of the current room and
  accepted only from them.
- The leave event reaches the chat listener (onSessionsLeft), so the typing
  indicator of a session which left the room is hidden at once.
- Typing is stopped and expired per session, not per participant.

The tests of WebSocketInstance use mockwebserver3 and hand the client and
the event bus to the production code through a mocked application
component, so the production classes need no test seam.

Assisted-by: Claude-Code:claude-sonnet-5-5
Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru>
@ToteMeiSter
ToteMeiSter force-pushed the fix/typing-indicator-room-sessions-v3 branch from 6eb2b01 to 8b914cb Compare October 22, 2026 09:19
@ToteMeiSter

Copy link
Copy Markdown
Contributor Author

Thanks for the review. All four points are handled in one commit, rebased on current master (f7c4841): 8b914cbdf.

Blocker: okhttp3.mockwebserver
WebSocketInstanceRoomSessionsTest now uses mockwebserver3 (Dispatcher, MockResponse.Builder().webSocketUpgrade(...), server.close()), like the tests of #6828. Server sockets are closed before server.close(), because close() of mockwebserver3 waits for open sockets. No okhttp3.mockwebserver import is left. compileGplayDebugUnitTestKotlin passes on top of master f7c4841; the tests of this PR (WebSocketInstanceRoomSessionsTest, RoomSessionRegistryTest, TypingParticipantTest, SignalingMessageReceiverSessionsLeftTest) pass; ktlint is clean; detekt gives the same 105 findings as master.

1. Test seams in production code — removed
WebSocketInstance has the constructor of master again (no testOkHttpClient, testEventBus, testConnectionHelper, no @JvmOverloads, no second branch in init). WebSocketConnectionHelper.java is identical to master. The test sets NextcloudTalkApplication.sharedApplication (as ChatViewModelUploadFileTest does) to a mocked application whose mocked NextcloudTalkApplicationComponent answers inject(WebSocketInstance) and inject(WebSocketConnectionHelper) with the test's OkHttpClient and EventBus. The tests run against the real production constructor and init.

2. Typing from a federated participant
What I checked in code:

  • The Android client does not distinguish federated sessions. RoomSessionRegistry.onJoin reads only sessionid, userid and user.displayname of each entry of the join list. It does not look at federated.
  • HPB sends federated sessions as normal entries of the join event (nextcloud-spreed-signaling at 274618cbe: server/room.go:432-444 fills SessionId, UserId, User and Federated = (ClientType == federation) for the list sent to a joining client; server/room.go:665-684 does the same for a session which joins later; the field is federated in api/signaling.go:1292-1299).
  • In a federated conversation the local HPB connects to the remote HPB with a FederationClient and forwards the room events to the client. server/federation.go:838-848 forwards join events and replaces only the own session id; :849-860 does the same for leave. So the sessions of the remote room reach the client as join in the same websocket and land in the registry.
  • Typing is a signaling message between sessions (MessageInputFragment.kt:985-986, ChatActivity.kt:566). Spreed (PHP) is not involved: SignalingController::getFederationSettings (lib/Controller/SignalingController.php:348) only returns the federation settings which the client sends in the room join.

What is not verified (needs a manual test, I have no federated setup): that a typing message addressed to a federated session id is routed by the HPBs in both directions, and which display name HPB sends in user for a federated session (if it sends none, the indicator shows "Guest", as for any session without a name). Test: two servers with HPB, a federated conversation, user A on the host, user B on the remote; A and B type in turn; check that each sees the other's indicator with the right name.

3. Typing between room join and the join list
Confirmed. A session becomes known only from the join list, and join events are ignored while no room is known (RoomSessionRegistry.kt:65-66). A startedTyping which arrives before the list is dropped (ChatActivity.kt:566). It is not lost for good: the sender resends startedTyping while it keeps typing, at most every 10 s (MessageInputFragment.kt, TYPING_DURATION_TO_SEND_NEXT_TYPING_MESSAGE). So the indicator can appear with a delay of up to 10 s after joining. I kept this on purpose: accepting typing from an unknown session was the cause of the "Guest is typing" bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants