regression: Adjust close connection condition

[juliajforesti]

Proposed changes (including videos or screenshots)

adjust user.forceLogout event condition for closing the connection when passing the sessionId

Issue(s)

Steps to test or reproduce

Further comments

root cause: #39383
CORE-1847

Summary by CodeRabbit

Release Notes

• Bug Fixes
  • Improved handling of forced user logout operations to ensure proper session termination when session identifiers are specified.

[dionisio-bot]

Looks like this PR is ready to merge! 🎉
If you have any trouble, please check the PR guidelines

[changeset-bot]

⚠️ No Changeset found

Latest commit: 1fd9a5e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[coderabbitai]

Walkthrough

The user.forceLogout handler in DDPStreamer was refactored to use an early return pattern when sessionId is provided, preventing subsequent userId-based logout logic from executing. This changes how the handler processes logout requests based on sessionId presence.

Changes

Cohort / File(s)	Summary
DDP Streamer Logout Logic ee/apps/ddp-streamer/src/DDPStreamer.ts	Modified the user.forceLogout handler's conditional flow to return early when sessionId is provided, preventing fallthrough to userId-based logout logic that previously occurred in all cases.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• chore: properly close websocket connection on device logout #39383: Modifies similar sessionId-based WebSocket closing logic in user.forceLogout handling across different services (DDPStreamer/MeteorService).

Suggested labels

type: chore, area: authentication

Suggested reviewers

• sampaiodiego
• tassoevan

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'regression: Adjust close connection condition' accurately describes the core change in the pull request, which modifies the conditional logic for closing connections in the forceLogout handler.

_{✏️ Tip: You can configure your own custom pre-merge checks in the 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]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.89%. Comparing base (4f43a85) to head (1fd9a5e).
⚠️ Report is 8 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #39470      +/-   ##
===========================================
- Coverage    70.94%   70.89%   -0.06%     
===========================================
  Files         3196     3196              
  Lines       113288   113288              
  Branches     20542    20509      -33     
===========================================
- Hits         80376    80319      -57     
- Misses       30865    30918      +53     
- Partials      2047     2051       +4     

Flag	Coverage Δ
e2e	60.43% <ø> (-0.04%)	⬇️
e2e-api	47.72% <ø> (-1.06%)	⬇️
unit	71.61% <ø> (-0.02%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 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 1 file

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@ee/apps/ddp-streamer/src/DDPStreamer.ts`:
- Around line 53-60: Add regression tests for the new (uid, sessionId) behavior
in DDPStreamer: create two websocket clients registered in clientMap with the
same uid but different connection.id values, then trigger the 'user.forceLogout'
event on the streamer with both uid+matching sessionId and uid+non-matching
sessionId cases; assert that the client whose client.connection.id equals the
sessionId is closed and the other remains open. Target the logic paths around
onEvent('user.forceLogout'), clientMap, wss?.clients, client?.connection.id and
client?.userId to ensure sessionId is authoritative when present and that
uid-only behavior remains unchanged. Ensure tests cover both matching and
non-matching sessionId while uid is provided so future changes cannot regress
this contract.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9bc4e234-10a6-4309-88eb-fd12549e4fb5

📥 Commits

Reviewing files that changed from the base of the PR and between 4f43a85 and 1fd9a5e.

📒 Files selected for processing (1)

• ee/apps/ddp-streamer/src/DDPStreamer.ts

📜 Review details

⏰ 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 context used

📓 Path-based instructions (1)

**/*.{ts,tsx,js}

📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)

**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation

Files:

• ee/apps/ddp-streamer/src/DDPStreamer.ts

🧠 Learnings (2)

📚 Learning: 2026-02-26T19:25:44.063Z

Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.

Applied to files:

• ee/apps/ddp-streamer/src/DDPStreamer.ts

📚 Learning: 2026-02-26T19:25:44.063Z

Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.

Applied to files:

• ee/apps/ddp-streamer/src/DDPStreamer.ts

[ricardogarim]

@juliajforesti, shouldn’t it be something like this #39116?

[ricardogarim]

/jira CORE-1847

[juliajforesti]

@juliajforesti, shouldn’t it be something like this #39116?

@ricardogarim The logic is slightly different, although I believe the result is the same...
In my case, I'm making the sessionId, when present, the ruler: if there is a match on the session id, it should be terminated and exit the loop - there won't be another session match, even if there's a user match.
In your case, the loop keeps verifying - unnecessarily, imo.