-
Notifications
You must be signed in to change notification settings - Fork 13k
Release 7.13.2 #37881
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.
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
Release 7.13.2 #37881
Conversation
🦋 Changeset detectedLatest commit: aba62ef The changes in this PR will be included in the next version bump. This PR includes changesets to release 41 packages
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 |
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
WalkthroughAdds heartbeat typings and a Heartbeat class; wires heartbeat-triggered presence updates in Meteor startup and DDP client; implements PresenceReaper to detect/remove stale sessions (with tests); exposes Presence.updateConnection API and IPresence method; exposes user field projections and adjusts query validation; adds changeset files. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as DDP Client
participant Meteor as Meteor Server (session.heartbeat)
participant PresenceSvc as Presence Service
participant DB as UsersSessions (DB)
Note over Client,Meteor: Message / Heartbeat path
Client->>Meteor: send message / heartbeat
Meteor->>Meteor: session.heartbeat.messageReceived()
Meteor->>PresenceSvc: updateConnection(uid, connectionId)
activate PresenceSvc
PresenceSvc->>DB: update UsersSessions._updatedAt for connection
DB-->>PresenceSvc: update result
PresenceSvc-->>Meteor: return { uid, connectionId }
deactivate PresenceSvc
Note over PresenceSvc,DB: Reaper periodic flow
loop every 60s
PresenceSvc->>PresenceSvc: PresenceReaper.run()
PresenceSvc->>DB: find sessions with _updatedAt <= cutoff
DB-->>PresenceSvc: cursor/documents
PresenceSvc->>DB: bulkWrite to pull stale connections (batched)
DB-->>PresenceSvc: bulkWrite result
PresenceSvc->>PresenceSvc: onUpdate(userIds) -> handleReaperUpdates
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
…tale (#37883) Co-authored-by: Matheus Cardoso <5606812+cardoso@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
ee/packages/presence/src/lib/PresenceReaper.spec.ts (1)
68-92: Good test coverage for PresenceReaper scenarios.The tests correctly validate:
- Empty cursor handling
- Stale connection detection
- Batch processing with correct grouping
- Mixed connection states
Consider adding assertions on
bulkWriteMockto verify the actual database operations match expectations (e.g., correct$pulloperations with the expected connection IDs and cutoff dates).🔎 Example assertion for bulkWrite verification
expect(bulkWriteMock).toHaveBeenCalledWith( expect.arrayContaining([ expect.objectContaining({ updateOne: expect.objectContaining({ filter: { _id: 'user-789' }, }), }), ]), );ee/packages/presence/src/lib/PresenceReaper.ts (1)
108-131: Race condition protection is well implemented.The
$pulloperation correctly includes bothid: { $in: plan.removeIds }and_updatedAt: { $lte: plan.cutoffDate }, ensuring only connections that haven't been refreshed since the scan are removed.Consider wrapping
bulkWritein a try-catch to preventonUpdatefrom being called if the database operation fails:🔎 Suggested error handling
private async flushBatch(changeMap: Map<string, ReaperPlan>): Promise<void> { const operations = []; // ... build operations ... if (isNonEmptyArray(operations)) { - await UsersSessions.col.bulkWrite(operations); - await this.onUpdate(operations.map((op) => op.updateOne.filter._id)); + try { + await UsersSessions.col.bulkWrite(operations); + await this.onUpdate(operations.map((op) => op.updateOne.filter._id)); + } catch (err) { + console.error('[PresenceReaper] flushBatch error:', err); + throw err; + } } }
📜 Review details
Configuration used: Organization 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.
📒 Files selected for processing (9)
.changeset/spicy-nails-design.md(1 hunks)apps/meteor/definition/externals/meteor/ddp-common.d.ts(1 hunks)apps/meteor/definition/externals/meteor/meteor.d.ts(2 hunks)apps/meteor/ee/server/startup/presence.ts(1 hunks)ee/apps/ddp-streamer/src/Client.ts(5 hunks)ee/packages/presence/src/Presence.ts(5 hunks)ee/packages/presence/src/lib/PresenceReaper.spec.ts(1 hunks)ee/packages/presence/src/lib/PresenceReaper.ts(1 hunks)packages/core-services/src/types/IPresence.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .changeset/spicy-nails-design.md
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{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/ee/server/startup/presence.tspackages/core-services/src/types/IPresence.tsee/packages/presence/src/lib/PresenceReaper.spec.tsapps/meteor/definition/externals/meteor/ddp-common.d.tsee/packages/presence/src/Presence.tsee/packages/presence/src/lib/PresenceReaper.tsapps/meteor/definition/externals/meteor/meteor.d.tsee/apps/ddp-streamer/src/Client.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.tsextension for test files (e.g.,login.spec.ts)
Files:
ee/packages/presence/src/lib/PresenceReaper.spec.ts
🧠 Learnings (10)
📓 Common learnings
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.
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37408
File: apps/meteor/client/views/admin/ABAC/useRoomAttributeOptions.tsx:53-69
Timestamp: 2025-11-10T19:06:20.146Z
Learning: In the Rocket.Chat repository, do not provide suggestions or recommendations about code sections marked with TODO comments. The maintainers have already identified these as future work and external reviewers lack the full context about implementation plans and timing.
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37547
File: packages/i18n/src/locales/en.i18n.json:634-634
Timestamp: 2025-11-19T12:32:29.696Z
Learning: Repo: RocketChat/Rocket.Chat
Context: i18n workflow
Learning: In this repository, new translation keys should be added to packages/i18n/src/locales/en.i18n.json only; other locale files are populated via the external translation pipeline and/or fall back to English. Do not request adding the same key to all locale files in future reviews.
📚 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:
ee/packages/presence/src/lib/PresenceReaper.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 : Ensure clean state for each test execution in Playwright tests
Applied to files:
ee/packages/presence/src/lib/PresenceReaper.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:
ee/packages/presence/src/lib/PresenceReaper.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:
ee/packages/presence/src/lib/PresenceReaper.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:
ee/packages/presence/src/lib/PresenceReaper.spec.ts
📚 Learning: 2025-12-10T21:00:43.645Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37091
File: ee/packages/abac/jest.config.ts:4-7
Timestamp: 2025-12-10T21:00:43.645Z
Learning: Adopt the monorepo-wide Jest testMatch pattern: <rootDir>/src/**/*.spec.{ts,js,mjs} (represented here as '**/src/**/*.spec.{ts,js,mjs}') to ensure spec files under any package's src directory are picked up consistently across all packages in the Rocket.Chat monorepo. Apply this pattern in jest.config.ts for all relevant packages to maintain uniform test discovery.
Applied to files:
ee/packages/presence/src/lib/PresenceReaper.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:
ee/packages/presence/src/lib/PresenceReaper.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:
ee/packages/presence/src/lib/PresenceReaper.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:
ee/packages/presence/src/lib/PresenceReaper.spec.ts
🧬 Code graph analysis (6)
apps/meteor/ee/server/startup/presence.ts (1)
ee/packages/presence/src/Presence.ts (1)
Presence(12-334)
ee/packages/presence/src/lib/PresenceReaper.spec.ts (2)
ee/packages/presence/src/lib/PresenceReaper.ts (1)
PresenceReaper(27-132)packages/models/src/index.ts (1)
registerModel(138-138)
ee/packages/presence/src/Presence.ts (2)
ee/packages/presence/src/lib/PresenceReaper.ts (1)
PresenceReaper(27-132)packages/models/src/index.ts (1)
UsersSessions(216-216)
ee/packages/presence/src/lib/PresenceReaper.ts (1)
packages/models/src/index.ts (1)
UsersSessions(216-216)
apps/meteor/definition/externals/meteor/meteor.d.ts (1)
ee/apps/ddp-streamer/src/configureServer.ts (1)
server(10-10)
ee/apps/ddp-streamer/src/Client.ts (1)
ee/packages/presence/src/Presence.ts (1)
Presence(12-334)
🔇 Additional comments (10)
packages/core-services/src/types/IPresence.ts (1)
16-16: LGTM!The new
updateConnectionmethod signature is consistent with existing methods likenewConnectionandremoveConnection, using the same return type pattern.ee/apps/ddp-streamer/src/Client.ts (2)
185-195: Correct implementation of heartbeat-based presence tracking.The logic properly ensures:
- Early return if packet was already seen or no userId (prevents duplicate updates)
- Sets
_seenPacket = truebefore the async call to prevent races- Error handling with logged context
206-211: Clean integration with the idle/heartbeat cycle.The conditional
_seenPacket = falsereset only for authenticated users (this.userIdexists) correctly scopes presence tracking to logged-in connections.ee/packages/presence/src/lib/PresenceReaper.ts (1)
45-53: Lifecycle management is correct.The
runningguard prevents multiple intervals, and the 60-second interval is reasonable for reaping stale connections.apps/meteor/definition/externals/meteor/meteor.d.ts (1)
42-47: Good improvement replacinganywith proper types.The typed
serverstructure enables compile-time checking for the heartbeat-related code inpresence.ts.DDPCommon.Heartbeatincludes both the_seenPacketproperty andmessageReceived()method accessed in the presence startup code.apps/meteor/ee/server/startup/presence.ts (1)
43-51: Code correctly implements presence tracking via Heartbeat interception.The implementation is sound. While
_seenPacketfollows underscore naming convention, it's explicitly declared as a public property in Meteor's DDPCommon.Heartbeat class type definitions, making it part of the documented API. The pattern correctly binds the original handler, handles errors gracefully, and achieves efficient presence detection by triggering updates only on the first packet of each heartbeat cycle. No changes required.apps/meteor/definition/externals/meteor/ddp-common.d.ts (1)
18-80: LGTM! Well-structured type definitions.The HeartbeatOptions type and Heartbeat class declarations are accurate and complete. The types correctly model the heartbeat lifecycle management with appropriate timer handle types and callback signatures.
ee/packages/presence/src/Presence.ts (3)
34-38: LGTM! Reaper properly initialized.The PresenceReaper is configured with reasonable thresholds (5-minute staleness) and batch size (500), with the callback correctly bound to the instance method.
84-85: LGTM! Clean lifecycle integration and error handling.The reaper is properly started and stopped in the lifecycle hooks. The
handleReaperUpdatesmethod usesPromise.allSettledto handle batch updates gracefully, with appropriate logging for successes and failures.Also applies to: 102-117, 119-120
168-188: LGTM! Correct implementation of connection timestamp update.The
updateConnectionmethod correctly uses MongoDB's positional operator to update the matched connection's timestamp, then triggers presence recalculation. The early return when nothing is modified avoids unnecessary work.
…#37887) Co-authored-by: Matheus Cardoso <5606812+cardoso@users.noreply.github.com>
Co-authored-by: Julio Araujo <julio.araujo@rocket.chat> Co-authored-by: Ricardo Garim <rswarovsky@gmail.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #37881 +/- ##
==========================================
+ Coverage 67.78% 67.80% +0.01%
==========================================
Files 3445 3445
Lines 113935 113928 -7
Branches 20844 20841 -3
==========================================
+ Hits 77233 77245 +12
+ Misses 34587 34566 -21
- Partials 2115 2117 +2
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Co-authored-by: Rodrigo Nascimento <234261+rodrigok@users.noreply.github.com>
There was a problem hiding this 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: Organization 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.
📒 Files selected for processing (3)
.github/actions/build-docker/action.yml(1 hunks).github/actions/setup-node/action.yml(2 hunks)apps/meteor/.docker/Dockerfile.alpine(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
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.
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37408
File: apps/meteor/client/views/admin/ABAC/useRoomAttributeOptions.tsx:53-69
Timestamp: 2025-11-10T19:06:20.146Z
Learning: In the Rocket.Chat repository, do not provide suggestions or recommendations about code sections marked with TODO comments. The maintainers have already identified these as future work and external reviewers lack the full context about implementation plans and timing.
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37547
File: packages/i18n/src/locales/en.i18n.json:634-634
Timestamp: 2025-11-19T12:32:29.696Z
Learning: Repo: RocketChat/Rocket.Chat
Context: i18n workflow
Learning: In this repository, new translation keys should be added to packages/i18n/src/locales/en.i18n.json only; other locale files are populated via the external translation pipeline and/or fall back to English. Do not request adding the same key to all locale files in future reviews.
🔇 Additional comments (4)
.github/actions/setup-node/action.yml (2)
54-55: Cache key improvements look good.The inclusion of
.yarnrc.ymlin the cache key ensures that yarn configuration changes properly invalidate the cache. The version bump to-v5will invalidate existing caches, which is appropriate given the architecture configuration changes introduced in this PR.
78-83: Architecture configuration aligns with PR objective.This configuration ensures that both arm64 and x64 (amd64) binaries are fetched, which should resolve the missing amd64 binaries issue mentioned in the PR objective. The step is correctly positioned before
yarn installand appropriately conditional oninputs.install.Note: This will increase installation time and cache size as binaries for multiple architectures will be downloaded.
.github/actions/build-docker/action.yml (1)
87-93: Verify sharp, pinyin, and esbuild binary removal patterns work correctly against actual bundle structure.The
findcommands use specific patterns to remove unwanted platform binaries, but these need validation against the actual directory names created duringnpm install:
- Line 87 (sharp): The pattern
*-linuxmusl-${swc_arch}should match musl-variant distributions- Line 89 (pinyin): The pattern
*-linux-${swc_arch}-*needs verification for correct napi-rs naming- Lines 91, 93 (esbuild): The pattern
*-${swc_arch}should correctly match esbuild's platform packagesIf patterns don't match the actual directory/package names installed, the wrong binaries may remain, potentially causing runtime failures. Consider testing these patterns against a sample bundle or adding verbose logging to confirm what directories are being matched and removed.
apps/meteor/.docker/Dockerfile.alpine (1)
11-12: Good simplification that correctly addresses the binary installation issue.The removal of custom sharp handling in favor of standard
npm install --omit=devis the right approach. Sharp is a runtime dependency (used in FileUpload.ts, emoji-custom, assets.ts, and avatar utilities), and this change correctly handles it by:
- Running
npm install --omit=devin the builder stage where build tools (python3, make, g++, libc6-compat) are available to compile native modules- Copying the compiled binaries to the final stage in the COPY command
This leverages Docker's multi-stage build pattern properly and lets npm handle platform-specific binaries automatically, which resolves the amd64 binary issue mentioned in the PR.
| export DENO_VERSION="${{ inputs.deno-version }}" | ||
| # Removes unnecessary swc cores to reduce image sized | ||
| # Removes unnecessary swc cores and sharp binaries to reduce image size |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update comment to include all removed binaries.
The comment mentions only "swc cores and sharp binaries" but the code also removes pinyin (line 89) and esbuild binaries (lines 91, 93). Update the comment to reflect all removal operations for clarity.
🔎 Suggested fix
- # Removes unnecessary swc cores and sharp binaries to reduce image size
+ # Removes unnecessary swc cores, sharp, pinyin, and esbuild binaries to reduce image size📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Removes unnecessary swc cores and sharp binaries to reduce image size | |
| # Removes unnecessary swc cores, sharp, pinyin, and esbuild binaries to reduce image size |
🤖 Prompt for AI Agents
.github/actions/build-docker/action.yml around line 78: the existing comment
only mentions "swc cores and sharp binaries" but the removal block also removes
pinyin and esbuild binaries; update the comment to list all removed items (swc
cores, sharp, pinyin, and esbuild binaries) or use a generic phrase like
"unnecessary native binaries (swc cores, sharp, pinyin, esbuild)" so the comment
accurately reflects the cleanup operations.
Summary by CodeRabbit
Chores
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.
You can see below a preview of the release change log:
7.13.2
Engine versions
22.16.01.43.55, 6, 7, 81.58.0Patch Changes
Bump @rocket.chat/meteor version.
(#37876 by @dionisio-bot) Security Hotfix (https://docs.rocket.chat/docs/security-fixes-and-updates)
(#37883 by @dionisio-bot) Ensures presence stays accurate by refreshing connections on heartbeats and removing stale sessions.
Updated dependencies [eef2b39]: