Skip to content

Conversation

@juliajforesti
Copy link
Contributor

@juliajforesti juliajforesti commented Oct 2, 2025

ARCH-1736

Proposed changes (including videos or screenshots)

Issue(s)

Steps to test or reproduce

Further comments

Summary by CodeRabbit

  • Removals

    • Removed ECDH-based encryption functionality and associated proxy service.
    • Removed related API endpoints and configuration settings.
  • Chores

    • Removed DDP package reference from package management.
    • Added database migration to clean up legacy ECDH settings.
  • Documentation

    • Removed ECDH proxy service documentation.

✏️ Tip: You can customize this high-level summary in your review settings.

@juliajforesti juliajforesti added this to the 8.0.0 milestone Oct 2, 2025
@dionisio-bot
Copy link
Contributor

dionisio-bot bot commented Oct 2, 2025

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

@changeset-bot
Copy link

changeset-bot bot commented Oct 2, 2025

🦋 Changeset detected

Latest commit: 4ee8726

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 39 packages
Name Type
@rocket.chat/rest-typings Major
@rocket.chat/meteor Major
@rocket.chat/api-client Patch
@rocket.chat/core-services Patch
@rocket.chat/ddp-client Patch
@rocket.chat/http-router Patch
@rocket.chat/models Patch
@rocket.chat/ui-contexts Major
@rocket.chat/web-ui-registration Major
@rocket.chat/account-service Patch
@rocket.chat/authorization-service Patch
@rocket.chat/ddp-streamer Patch
@rocket.chat/federation-matrix Patch
@rocket.chat/omnichannel-services Patch
@rocket.chat/presence Patch
rocketchat-services Patch
@rocket.chat/omnichannel-transcript Patch
@rocket.chat/presence-service Patch
@rocket.chat/queue-worker Patch
@rocket.chat/network-broker Patch
@rocket.chat/omni-core-ee Patch
@rocket.chat/livechat Patch
@rocket.chat/mock-providers Patch
@rocket.chat/cron Patch
@rocket.chat/instance-status Patch
@rocket.chat/omni-core Patch
@rocket.chat/media-calls Patch
@rocket.chat/uikit-playground Patch
@rocket.chat/fuselage-ui-kit Major
@rocket.chat/gazzodown Major
@rocket.chat/ui-avatar Major
@rocket.chat/ui-client Major
@rocket.chat/ui-video-conf Major
@rocket.chat/ui-voip Major
@rocket.chat/core-typings Major
@rocket.chat/apps Patch
@rocket.chat/model-typings Patch
@rocket.chat/license Patch
@rocket.chat/pdf-worker Patch

Not sure what this means? Click here to learn what changesets are.

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 2, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

Removes ECDH encryption functionality and related infrastructure across the codebase, including ECDH session classes, proxy service, client-side integration, settings, and the rocketchat-ddp Meteor package. Adds a database migration to clean up the ECDH_Enabled setting and updates type definitions to remove references to ECDH endpoints.

Changes

Cohort / File(s) Summary
Release & Metadata
.changeset/fair-dryers-behave.md
New changeset file documenting major version bumps for @rocket.chat/rest-typings and @rocket.chat/meteor and removal of ECDH functionality.
ECDH Core Classes
apps/meteor/app/ecdh/Session.ts, apps/meteor/app/ecdh/client/ClientSession.ts, apps/meteor/ee/app/ecdh/server/ServerSession.ts
Removed ECDH session classes for X25519 key exchange and sodium-plus encryption/decryption (Session, ClientSession, ServerSession) and associated crypto operations.
ECDH Proxy Service
apps/meteor/ee/server/services/ecdh-proxy/ECDHProxy.ts, apps/meteor/ee/server/services/ecdh-proxy/lib/server.ts, apps/meteor/ee/server/services/ecdh-proxy/service.ts, apps/meteor/ee/server/services/ecdh-proxy/README.md
Removed ECDHProxy service class, HTTP/WebSocket proxy implementation, session handling logic, and documentation.
Client-side ECDH Integration
apps/meteor/client/ecdh.ts, apps/meteor/client/main.ts
Removed ECDH session establishment, Meteor socket message interception/encryption, and dynamic import from login chain.
Meteor Package Removal (rocketchat-ddp)
apps/meteor/.meteor/packages, apps/meteor/.meteor/versions, apps/meteor/packages/rocketchat-ddp/client/index.js, apps/meteor/packages/rocketchat-ddp/package.js
Removed rocketchat:ddp package, its manifest, version entry, and ClientStream monkey-patching for connection management.
Settings & Configuration
apps/meteor/server/settings/general.ts, apps/meteor/app/ui-master/server/scripts.ts
Removed ECDH_Enabled setting from General settings and removed ECDH_Enabled from watched keys in runtime script injection.
Type Definitions
apps/meteor/definition/externals/global.d.ts, apps/meteor/definition/externals/meteor/meteor.d.ts, packages/rest-typings/src/default/index.ts
Removed ECDH_Enabled optional property from Window interface, allowConnection method from IMeteorConnection._stream, and ecdh_proxy/initEncryptedSession endpoint from DefaultEndpoints.
API Route Removal
apps/meteor/app/api/server/default/info.ts
Removed POST route registration for ecdh_proxy/initEncryptedSession API endpoint.
Database Migration
apps/meteor/server/startup/migrations/index.ts, apps/meteor/server/startup/migrations/v330.ts
Added v330 migration import and new migration file to delete ECDH_Enabled setting from Settings collection.
Test Updates
apps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.ts, apps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.ts
Added injectInitialData teardown in e2ee-key-reset test and removed initial data management from e2ee-legacy-format test.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–25 minutes

  • Scope: 20+ files across diverse modules (crypto, services, client, settings, types, migrations)
  • Complexity: Low—primarily straightforward deletions, though spread widely across different architectural layers
  • Heterogeneity: Mixed change types (complete class/module removal, package deletion, setting removal, API route removal, migrations, test updates)

Areas requiring extra attention:

  • Verify no orphaned imports or broken references to removed ECDH classes and packages throughout codebase
  • Confirm migration v330 is correctly ordered and integrated into the migration sequence
  • Validate that all ECDH-related type definitions and settings have been exhaustively removed
  • Check test updates are sufficient to maintain E2E encryption test coverage without ECDH

Suggested reviewers

  • cardoso
  • dougfabris

Poem

🐰 A hop, a skip, ECDH encrypted no more,
The crypto keys vanish, the sessions restore,
Clean migrations sweep through each setting in place,
The rocketchat-ddp vanishes without a trace!

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title 'chore!: remove ecdh' clearly summarizes the main objective of removing ECDH functionality across the codebase.
Linked Issues check ✅ Passed The pull request comprehensively removes all ECDH-related code, settings, services, migrations, and type definitions as required by ARCH-1736.
Out of Scope Changes check ✅ Passed All changes are directly related to removing ECDH functionality; no unrelated modifications detected outside the stated objective.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

📜 Recent 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 fa01b89 and 4ee8726.

📒 Files selected for processing (24)
  • .changeset/fair-dryers-behave.md (1 hunks)
  • apps/meteor/.meteor/packages (0 hunks)
  • apps/meteor/.meteor/versions (0 hunks)
  • apps/meteor/app/api/server/default/info.ts (0 hunks)
  • apps/meteor/app/ecdh/Session.ts (0 hunks)
  • apps/meteor/app/ecdh/client/ClientSession.ts (0 hunks)
  • apps/meteor/app/ui-master/server/scripts.ts (1 hunks)
  • apps/meteor/client/ecdh.ts (0 hunks)
  • apps/meteor/client/main.ts (0 hunks)
  • apps/meteor/definition/externals/global.d.ts (0 hunks)
  • apps/meteor/definition/externals/meteor/meteor.d.ts (0 hunks)
  • apps/meteor/ee/app/ecdh/server/ServerSession.ts (0 hunks)
  • apps/meteor/ee/server/services/ecdh-proxy/ECDHProxy.ts (0 hunks)
  • apps/meteor/ee/server/services/ecdh-proxy/README.md (0 hunks)
  • apps/meteor/ee/server/services/ecdh-proxy/lib/server.ts (0 hunks)
  • apps/meteor/ee/server/services/ecdh-proxy/service.ts (0 hunks)
  • apps/meteor/packages/rocketchat-ddp/client/index.js (0 hunks)
  • apps/meteor/packages/rocketchat-ddp/package.js (0 hunks)
  • apps/meteor/server/settings/general.ts (0 hunks)
  • apps/meteor/server/startup/migrations/index.ts (1 hunks)
  • apps/meteor/server/startup/migrations/v330.ts (1 hunks)
  • apps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.ts (2 hunks)
  • apps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.ts (1 hunks)
  • packages/rest-typings/src/default/index.ts (0 hunks)

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
Copy link

codecov bot commented Oct 2, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.24%. Comparing base (d072acd) to head (ac3ad33).

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff               @@
##           release-8.0.0   #37123   +/-   ##
==============================================
  Coverage          70.24%   70.24%           
==============================================
  Files               2999     2999           
  Lines             102301   102297    -4     
  Branches           18221    18224    +3     
==============================================
  Hits               71861    71861           
+ Misses             28576    28569    -7     
- Partials            1864     1867    +3     
Flag Coverage Δ
unit 71.33% <ø> (-0.01%) ⬇️

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.

@juliajforesti juliajforesti force-pushed the chore/remove-ecdh branch 2 times, most recently from b58f2a5 to e265ab7 Compare October 6, 2025 13:52
@ggazzo ggazzo force-pushed the release-8.0.0 branch 11 times, most recently from dd959e2 to 589f2a1 Compare November 6, 2025 18:06
@ggazzo ggazzo force-pushed the release-8.0.0 branch 5 times, most recently from 481c8af to 94daba9 Compare November 18, 2025 18:57
@github-actions
Copy link
Contributor

github-actions bot commented Dec 1, 2025

📦 Docker Image Size Report

📈 Changes

Service Current Baseline Change Percent
sum of all images 1.1GiB 1.1GiB +8.0MiB
rocketchat 357MiB 349MiB +8.4MiB
omnichannel-transcript-service 132MiB 132MiB -105KiB
queue-worker-service 132MiB 132MiB -107KiB
ddp-streamer-service 126MiB 126MiB -58KiB
account-service 113MiB 113MiB -58KiB
authorization-service 111MiB 111MiB -55KiB
presence-service 111MiB 111MiB -60KiB

📊 Historical Trend

---
config:
  theme: "dark"
  xyChart:
    width: 900
    height: 400
---
xychart
  title "Image Size Evolution by Service (Last 30 Days + This PR)"
  x-axis ["11/15 22:28", "11/16 01:28", "11/17 23:50", "11/18 22:53", "11/19 23:02", "11/21 16:49", "11/24 17:34", "11/27 22:32", "11/28 19:05", "12/01 23:01", "12/02 21:57", "12/03 21:00", "12/04 18:17", "12/05 21:56", "12/08 20:15", "12/09 22:17", "12/10 03:19", "12/10 03:22 (PR)"]
  y-axis "Size (GB)" 0 --> 0.5
  line "account-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
  line "authorization-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
  line "ddp-streamer-service" [0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12]
  line "omnichannel-transcript-service" [0.14, 0.14, 0.14, 0.14, 0.14, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13]
  line "presence-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
  line "queue-worker-service" [0.14, 0.14, 0.14, 0.14, 0.14, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13]
  line "rocketchat" [0.36, 0.36, 0.35, 0.35, 0.35, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.35]
Loading

Statistics (last 17 days):

  • 📊 Average: 1.5GiB
  • ⬇️ Minimum: 1.2GiB
  • ⬆️ Maximum: 1.6GiB
  • 🎯 Current PR: 1.1GiB
ℹ️ About this report

This report compares Docker image sizes from this build against the develop baseline.

  • Tag: pr-37123
  • Baseline: develop
  • Timestamp: 2025-12-10 03:22:09 UTC
  • Historical data points: 17

Updated: Wed, 10 Dec 2025 03:22:10 GMT

@ggazzo ggazzo force-pushed the release-8.0.0 branch 4 times, most recently from b7b6960 to 169b783 Compare December 2, 2025 22:46
@ggazzo ggazzo added the stat: QA assured Means it has been tested and approved by a company insider label Dec 10, 2025
@dionisio-bot dionisio-bot bot added the stat: ready to merge PR tested and approved waiting for merge label Dec 10, 2025
@ggazzo ggazzo merged commit 0fe2977 into release-8.0.0 Dec 10, 2025
10 of 11 checks passed
@ggazzo ggazzo deleted the chore/remove-ecdh branch December 10, 2025 13:01
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 88fd76d and fa01b89.

📒 Files selected for processing (24)
  • .changeset/fair-dryers-behave.md (1 hunks)
  • apps/meteor/.meteor/packages (0 hunks)
  • apps/meteor/.meteor/versions (0 hunks)
  • apps/meteor/app/api/server/default/info.ts (0 hunks)
  • apps/meteor/app/ecdh/Session.ts (0 hunks)
  • apps/meteor/app/ecdh/client/ClientSession.ts (0 hunks)
  • apps/meteor/app/ui-master/server/scripts.ts (1 hunks)
  • apps/meteor/client/ecdh.ts (0 hunks)
  • apps/meteor/client/main.ts (0 hunks)
  • apps/meteor/definition/externals/global.d.ts (0 hunks)
  • apps/meteor/definition/externals/meteor/meteor.d.ts (0 hunks)
  • apps/meteor/ee/app/ecdh/server/ServerSession.ts (0 hunks)
  • apps/meteor/ee/server/services/ecdh-proxy/ECDHProxy.ts (0 hunks)
  • apps/meteor/ee/server/services/ecdh-proxy/README.md (0 hunks)
  • apps/meteor/ee/server/services/ecdh-proxy/lib/server.ts (0 hunks)
  • apps/meteor/ee/server/services/ecdh-proxy/service.ts (0 hunks)
  • apps/meteor/packages/rocketchat-ddp/client/index.js (0 hunks)
  • apps/meteor/packages/rocketchat-ddp/package.js (0 hunks)
  • apps/meteor/server/settings/general.ts (0 hunks)
  • apps/meteor/server/startup/migrations/index.ts (1 hunks)
  • apps/meteor/server/startup/migrations/v329.ts (1 hunks)
  • apps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.ts (2 hunks)
  • apps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.ts (1 hunks)
  • packages/rest-typings/src/default/index.ts (0 hunks)
💤 Files with no reviewable changes (18)
  • packages/rest-typings/src/default/index.ts
  • apps/meteor/definition/externals/meteor/meteor.d.ts
  • apps/meteor/server/settings/general.ts
  • apps/meteor/definition/externals/global.d.ts
  • apps/meteor/ee/app/ecdh/server/ServerSession.ts
  • apps/meteor/client/ecdh.ts
  • apps/meteor/client/main.ts
  • apps/meteor/ee/server/services/ecdh-proxy/README.md
  • apps/meteor/.meteor/versions
  • apps/meteor/packages/rocketchat-ddp/package.js
  • apps/meteor/ee/server/services/ecdh-proxy/service.ts
  • apps/meteor/app/ecdh/Session.ts
  • apps/meteor/.meteor/packages
  • apps/meteor/app/api/server/default/info.ts
  • apps/meteor/app/ecdh/client/ClientSession.ts
  • apps/meteor/ee/server/services/ecdh-proxy/lib/server.ts
  • apps/meteor/ee/server/services/ecdh-proxy/ECDHProxy.ts
  • apps/meteor/packages/rocketchat-ddp/client/index.js
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{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:

  • apps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.ts
  • apps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.ts
  • apps/meteor/server/startup/migrations/v329.ts
  • apps/meteor/app/ui-master/server/scripts.ts
  • apps/meteor/server/startup/migrations/index.ts
**/*.spec.ts

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

**/*.spec.ts: Use descriptive test names that clearly communicate expected behavior in Playwright tests
Use .spec.ts extension for test files (e.g., login.spec.ts)

Files:

  • apps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.ts
  • apps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.ts
apps/meteor/tests/e2e/**/*.spec.ts

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

apps/meteor/tests/e2e/**/*.spec.ts: All test files must be created in apps/meteor/tests/e2e/ directory
Avoid using page.locator() in Playwright tests - always prefer semantic locators such as page.getByRole(), page.getByLabel(), page.getByText(), or page.getByTitle()
Use test.beforeAll() and test.afterAll() for setup/teardown in Playwright tests
Use test.step() for complex test scenarios to improve organization in Playwright tests
Group related tests in the same file
Utilize Playwright fixtures (test, page, expect) for consistency in test files
Prefer web-first assertions (toBeVisible, toHaveText, etc.) in Playwright tests
Use expect matchers for assertions (toEqual, toContain, toBeTruthy, toHaveLength, etc.) instead of assert statements in Playwright tests
Use page.waitFor() with specific conditions instead of hardcoded timeouts in Playwright tests
Implement proper wait strategies for dynamic content in Playwright tests
Maintain test isolation between test cases in Playwright tests
Ensure clean state for each test execution in Playwright tests
Ensure tests run reliably in parallel without shared state conflicts

Files:

  • apps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.ts
  • apps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.ts
apps/meteor/tests/e2e/**/*.{ts,spec.ts}

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

apps/meteor/tests/e2e/**/*.{ts,spec.ts}: Store commonly used locators in variables/constants for reuse
Follow Page Object Model pattern consistently in Playwright tests

Files:

  • apps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.ts
  • apps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.ts
🧠 Learnings (17)
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts

Applied to files:

  • apps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.ts
  • apps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.ts
  • apps/meteor/server/startup/migrations/index.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure clean state for each test execution in Playwright tests

Applied to files:

  • apps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.ts
  • apps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (`test`, `page`, `expect`) for consistency in test files

Applied to files:

  • apps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.ts
  • apps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Group related tests in the same file

Applied to files:

  • apps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.ts
  • apps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Maintain test isolation between test cases in Playwright tests

Applied to files:

  • apps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.ts
  • apps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : All test files must be created in `apps/meteor/tests/e2e/` directory

Applied to files:

  • apps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.ts
  • apps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `test.beforeAll()` and `test.afterAll()` for setup/teardown in Playwright tests

Applied to files:

  • apps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.ts
  • apps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `expect` matchers for assertions (`toEqual`, `toContain`, `toBeTruthy`, `toHaveLength`, etc.) instead of `assert` statements in Playwright tests

Applied to files:

  • apps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.ts
  • apps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `test.step()` for complex test scenarios to improve organization in Playwright tests

Applied to files:

  • apps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.ts
  • apps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/page-objects/**/*.ts : Utilize existing page objects pattern from `apps/meteor/tests/e2e/page-objects/`

Applied to files:

  • apps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.ts
  • apps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,spec.ts} : Follow Page Object Model pattern consistently in Playwright tests

Applied to files:

  • apps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.ts
  • apps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `page.waitFor()` with specific conditions instead of hardcoded timeouts in Playwright tests

Applied to files:

  • apps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Implement proper wait strategies for dynamic content in Playwright tests

Applied to files:

  • apps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.ts
  • apps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,spec.ts} : Store commonly used locators in variables/constants for reuse

Applied to files:

  • apps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.ts
📚 Learning: 2025-09-19T15:15:04.642Z
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 36991
File: apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts:219-221
Timestamp: 2025-09-19T15:15:04.642Z
Learning: The Federation_Matrix_homeserver_domain setting in apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts is part of the old federation system and is being deprecated/removed, so configuration issues with this setting should not be flagged for improvement.

Applied to files:

  • .changeset/fair-dryers-behave.md
  • apps/meteor/server/startup/migrations/v329.ts
  • apps/meteor/app/ui-master/server/scripts.ts
📚 Learning: 2025-10-16T21:09:51.816Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36942
File: apps/meteor/client/lib/e2ee/keychain.ts:148-156
Timestamp: 2025-10-16T21:09:51.816Z
Learning: In the RocketChat/Rocket.Chat repository, only platforms with native crypto.randomUUID() support are targeted, so fallback implementations for crypto.randomUUID() are not required in E2EE or cryptographic code.

Applied to files:

  • .changeset/fair-dryers-behave.md
📚 Learning: 2025-11-05T20:53:57.761Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37357
File: apps/meteor/ee/server/startup/federation.ts:39-74
Timestamp: 2025-11-05T20:53:57.761Z
Learning: In Rocket.Chat (apps/meteor/app/settings/server/CachedSettings.ts), the settings.watchMultiple() method immediately invokes its callback with current values if all requested settings exist in the store, then continues watching for subsequent changes. It does not wait for a setting to change before the first invocation.

Applied to files:

  • apps/meteor/app/ui-master/server/scripts.ts
🔇 Additional comments (5)
.changeset/fair-dryers-behave.md (1)

1-6: LGTM!

The changeset correctly specifies major version bumps for the affected packages, which is appropriate for removing ECDH functionality (a breaking change).

apps/meteor/server/startup/migrations/index.ts (1)

37-37: LGTM!

The import follows the established sequential pattern and is correctly placed.

apps/meteor/server/startup/migrations/v329.ts (1)

5-11: LGTM - Migration correctly removes the ECDH_Enabled setting.

The migration correctly uses deleteOne to remove the specific setting by _id. Note that this migration is non-reversible (no down() method), which is acceptable for cleanup of deprecated functionality.

apps/meteor/app/ui-master/server/scripts.ts (1)

53-59: Removal of ECDH_Enabled from watched settings is correct.

The change properly removes ECDH_Enabled from the observed settings list. The setting itself was deleted via migration v329.ts. Translation strings in i18n locale files are retained but do not affect functionality.

apps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.ts (1)

4-4: LGTM! Good addition for test isolation.

Adding injectInitialData() in afterEach ensures clean state between test executions and maintains proper test isolation, aligning with Playwright best practices.

Based on learnings, this change helps ensure clean state for each test execution in Playwright tests.

Also applies to: 39-39

import { BASE_API_URL } from '../config/constants';
import injectInitialData from '../fixtures/inject-initial-data';
import { Users, restoreState } from '../fixtures/userStates';
import { Users } from '../fixtures/userStates';
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 | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify cleanup strategy and injectInitialData usage in E2E encryption tests

echo "=== Checking for injectInitialData usage in E2E encryption tests ==="
rg -n -C3 'injectInitialData' apps/meteor/tests/e2e/e2e-encryption/

echo -e "\n=== Checking afterEach cleanup patterns in E2E encryption tests ==="
rg -n -A5 'test\.afterEach' apps/meteor/tests/e2e/e2e-encryption/

echo -e "\n=== Listing all test files in e2e-encryption directory ==="
fd -e spec.ts . apps/meteor/tests/e2e/e2e-encryption/

Repository: RocketChat/Rocket.Chat

Length of output: 7178


🏁 Script executed:

#!/bin/bash
# Read the target file to check current cleanup hooks

echo "=== Content of e2ee-legacy-format.spec.ts ==="
cat -n apps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.ts

echo -e "\n=== Checking for any cleanup hooks in e2ee-legacy-format.spec.ts ==="
rg -n 'test\.(after|before)' apps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.ts

Repository: RocketChat/Rocket.Chat

Length of output: 4370


Add cleanup hook to reset test state between executions.

This test file lacks an afterEach hook, while e2ee-key-reset.spec.ts uses test.afterEach() with injectInitialData() for cleanup. Since this test creates encrypted channels that persist across test runs, add a similar cleanup hook to ensure test isolation and prevent state leakage between parallel test executions.

test.afterEach(async () => {
	await injectInitialData();
});

Also add the import at the top:

import injectInitialData from '../fixtures/inject-initial-data';
🤖 Prompt for AI Agents
In apps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.ts around line
5, the test file lacks a cleanup hook to reset state between runs; add an import
for injectInitialData from '../fixtures/inject-initial-data' near the top (with
the other imports) and add a test.afterEach(async () => { await
injectInitialData(); }); after the tests to ensure encrypted channels and other
state are reset between executions.

ggazzo added a commit that referenced this pull request Dec 10, 2025
Co-authored-by: Guilherme Gazzo <guilherme@gazzo.xyz>
ggazzo added a commit that referenced this pull request Dec 15, 2025
Co-authored-by: Guilherme Gazzo <guilherme@gazzo.xyz>
ggazzo added a commit that referenced this pull request Dec 17, 2025
Co-authored-by: Guilherme Gazzo <guilherme@gazzo.xyz>
ggazzo added a commit that referenced this pull request Dec 17, 2025
Co-authored-by: Guilherme Gazzo <guilherme@gazzo.xyz>
ggazzo added a commit that referenced this pull request Dec 17, 2025
Co-authored-by: Guilherme Gazzo <guilherme@gazzo.xyz>
ggazzo added a commit that referenced this pull request Dec 18, 2025
Co-authored-by: Guilherme Gazzo <guilherme@gazzo.xyz>
ggazzo added a commit that referenced this pull request Dec 18, 2025
Co-authored-by: Guilherme Gazzo <guilherme@gazzo.xyz>
ggazzo added a commit that referenced this pull request Dec 18, 2025
Co-authored-by: Guilherme Gazzo <guilherme@gazzo.xyz>
ggazzo added a commit that referenced this pull request Dec 18, 2025
Co-authored-by: Guilherme Gazzo <guilherme@gazzo.xyz>
ggazzo added a commit that referenced this pull request Dec 19, 2025
Co-authored-by: Guilherme Gazzo <guilherme@gazzo.xyz>
ggazzo added a commit that referenced this pull request Dec 19, 2025
Co-authored-by: Guilherme Gazzo <guilherme@gazzo.xyz>
ggazzo added a commit that referenced this pull request Dec 19, 2025
Co-authored-by: Guilherme Gazzo <guilherme@gazzo.xyz>
ggazzo added a commit that referenced this pull request Dec 20, 2025
Co-authored-by: Guilherme Gazzo <guilherme@gazzo.xyz>
ggazzo added a commit that referenced this pull request Dec 20, 2025
Co-authored-by: Guilherme Gazzo <guilherme@gazzo.xyz>
ggazzo added a commit that referenced this pull request Dec 20, 2025
Co-authored-by: Guilherme Gazzo <guilherme@gazzo.xyz>
ggazzo added a commit that referenced this pull request Dec 20, 2025
Co-authored-by: Guilherme Gazzo <guilherme@gazzo.xyz>
dougfabris pushed a commit that referenced this pull request Dec 20, 2025
Co-authored-by: Guilherme Gazzo <guilherme@gazzo.xyz>
gaolin1 pushed a commit to gaolin1/medsense.webchat that referenced this pull request Jan 6, 2026
Co-authored-by: Guilherme Gazzo <guilherme@gazzo.xyz>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stat: QA assured Means it has been tested and approved by a company insider stat: ready to merge PR tested and approved waiting for merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants