-
Notifications
You must be signed in to change notification settings - Fork 13.1k
chore: Standardize eslint typescript rules #38584
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
Conversation
|
Looks like this PR is ready to merge! 🎉 |
|
WalkthroughThis pull request systematically adds Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ 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. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #38584 +/- ##
===========================================
- Coverage 70.45% 70.44% -0.02%
===========================================
Files 3174 3174
Lines 111003 111003
Branches 19991 19988 -3
===========================================
- Hits 78204 78192 -12
- Misses 30756 30764 +8
- Partials 2043 2047 +4
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
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.
No issues found across 35 files
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: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
ee/apps/queue-worker/src/service.ts (1)
19-19:⚠️ Potential issue | 🟡 MinorPre-existing typo in comment:
registeredpackagfe→registered package.packages/instance-status/src/index.ts (1)
31-56:⚠️ Potential issue | 🟠 MajorRace condition:
dropIndexandcreateIndexmay execute concurrently.When an existing index has a mismatched
expireAfterSeconds, line 39 firesdropIndexwithout awaiting, thensome()returnsfalse, causing line 49 to firecreateIndeximmediately. These two operations can overlap, and MongoDB may reject thecreateIndexif the old index hasn't been fully dropped yet. Since both promises arevoid-ed, the error is silently lost, leaving the collection without a proper TTL index.Previously these calls were presumably returned/awaited within the
.then()chain, maintaining sequential execution.Suggested fix: keep the index operations awaited within the chain
let createIndexes = async () => { - await InstanceStatusModel.col - .indexes() - .catch(() => []) - .then((result) => - result.some((index) => { - if (index.key && index.key._updatedAt === 1) { - if (index.expireAfterSeconds !== indexExpire && index.name) { - void InstanceStatusModel.col.dropIndex(index.name); - return false; - } - return true; - } - return false; - }), - ) - .then((created) => { - if (!created) { - void InstanceStatusModel.col.createIndex({ _updatedAt: 1 }, { expireAfterSeconds: indexExpire }); - } - }); + const existingIndexes = await InstanceStatusModel.col.indexes().catch(() => []); + const hasValidIndex = existingIndexes.some((index) => { + if (index.key && index.key._updatedAt === 1) { + return index.expireAfterSeconds === indexExpire; + } + return false; + }); + + const staleIndex = existingIndexes.find( + (index) => index.key && index.key._updatedAt === 1 && index.expireAfterSeconds !== indexExpire && index.name, + ); + if (staleIndex?.name) { + await InstanceStatusModel.col.dropIndex(staleIndex.name); + } + + if (!hasValidIndex) { + await InstanceStatusModel.col.createIndex({ _updatedAt: 1 }, { expireAfterSeconds: indexExpire }); + } createIndexes = async () => { // noop }; };ee/packages/ui-theming/src/hooks/useThemeMode.ts (1)
11-25:⚠️ Potential issue | 🟡 MinorReturn type of
setThemeis inaccurate — the returned function yields aPromise, notvoid.Line 11 declares the setter as
(value: ThemeMode) => () => void, but the updaters (line 17) now explicitly returnReturnType<typeof saveUserPreferences>(aPromise). Callers invoking the returned thunk receive an unhandled promise. This mismatch was likely pre-existing but is now surfaced by the type change on line 17.Suggested type alignment
-export const useThemeMode = (): [ThemeMode, (value: ThemeMode) => () => void, Themes] => { +export const useThemeMode = (): [ThemeMode, (value: ThemeMode) => () => ReturnType<typeof saveUserPreferences>, Themes] => {Note:
saveUserPreferencesis scoped inside the hook, so you may need to extract or inline the return type (e.g.,() => Promise<void>). Alternatively, if callers intentionally discard the promise, keep() => voidbut addvoidat call sites.packages/agenda/src/Agenda.ts (1)
180-188:⚠️ Potential issue | 🟡 Minor
void this.dbInit(collection)silently detaches index-creation errors fromawait database().
database()ispublic async, so external callers canawaitit. By voidingdbInit, any index-creation failure insidedbInitwill not propagate to callers ofdatabase(). The'error'event is still emitted, but callers awaitingdatabase()will see it resolve successfully even whendbInitfails.If this is intentional (relying solely on the event pattern), consider documenting it. Otherwise,
awaitwould preserve error propagation:Suggested fix
- void this.dbInit(collection); + await this.dbInit(collection);
🤖 Fix all issues with AI agents
In `@ee/apps/ddp-streamer/src/DDPStreamer.ts`:
- Around line 185-215: Presence.removeConnection calls in the LOGGEDOUT and
DISCONNECTED handlers are fire-and-forget so failures are swallowed; update
those handlers (the server.on callbacks for DDP_EVENTS.LOGGEDOUT and
DDP_EVENTS.DISCONNECTED) to handle errors from Presence.removeConnection by
either awaiting the promise or attaching a .catch(...) that logs the failure
(include context like userId, connection.id and nodeID) and ensure
updateConnections remains called as appropriate; reference the
Presence.removeConnection(...) calls in those handlers and add proper error
logging/handling consistent with how Presence.newConnection is awaited in the
LOGGED handler.
In `@ee/apps/omnichannel-transcript/src/service.ts`:
- Line 12: The top-level IIFE `void (async () => { ... })` in service.ts can
produce unhandled promise rejections; modify this bootstrap to append a
`.catch(...)` to the IIFE (or register `process.on('unhandledRejection', ...)`)
that logs the error (use the existing logger or `console.error`) and calls
`process.exit(1)` so any rejected await inside the async IIFE (e.g., DB
connection failures) is logged and the process exits cleanly.
In `@ee/apps/presence-service/src/service.ts`:
- Line 9: The top-level IIFE using "void (async () => { ... })()" swallows
startup rejections; update the entry to surface failures by removing the leading
void and handling rejections: invoke the async entry function and attach a
.catch handler that logs the error (e.g., via logger.error or console.error) and
calls process.exit(1). Ensure the error handler covers failures from
getConnection(), api.start(), and any awaited calls inside the IIFE so the
process crashes on fatal startup errors.
In `@ee/apps/queue-worker/src/service.ts`:
- Line 10: The startup uses an async IIFE prefixed with "void (async () => {"
which swallows thrown errors; replace this pattern so startup failures are not
ignored by invoking the IIFE and catching errors (e.g., call the IIFE with ())
and add a .catch handler that logs the error and calls process.exit(1); locate
the async IIFE started with "void (async () => {" in service.ts and change its
invocation to ensure any thrown exceptions are handled and cause the process to
exit.
In `@ee/packages/media-calls/src/sip/providers/IncomingSipCall.ts`:
- Line 172: The call to calleeAgent.onRemoteDescriptionChanged(this.call._id,
negotiationId) is currently void-ed which swallows rejections and prevents the
surrounding try/catch from performing cleanup (sending the SIP error response,
removing the negotiation, and hanging up); change this so the promise rejection
is handled: either await calleeAgent.onRemoteDescriptionChanged(this.call._id,
negotiationId) inside the existing try block so failures flow to the catch, or
if you must keep it non-blocking, attach a .catch that invokes the same recovery
steps (call sendSipErrorResponse with the same args, remove the negotiation from
the negotiations map with negotiations.delete(negotiationId), and call
this.call.hangup()) and rethrow or log appropriately so the main error-recovery
logic still runs.
In `@ee/packages/omnichannel-services/src/OmnichannelTranscript.ts`:
- Around line 378-380: The fire-and-forget call to pdfFailed is creating a risk
of unhandled promise rejections because pdfFailed performs async I/O
(createDirectMessage, sendMessage) and has no internal try/catch; replace the
current void this.pdfFailed({ details, e: error as Error, i18n }); with a
fire-and-forget invocation that attaches a .catch(...) to handle/log any error
(e.g. this.pdfFailed(...).catch(err => processLogger?.error('pdfFailed error',
err)) ) so failures inside pdfFailed are swallowed/logged; alternatively make
the call consistent with the other site that awaits pdfFailed by awaiting it if
the surrounding flow permits. Ensure references to pdfFailed,
createDirectMessage and sendMessage are preserved when adding the .catch or
switching to await.
In `@ee/packages/presence/src/Presence.ts`:
- Line 54: The call to validateAvailability on the Presence class is currently
invoked with "void this.validateAvailability()", which swallows promise
rejections from validateAvailability (which writes via
Settings.updateValueById). Change the invocation to properly handle errors:
either await this.validateAvailability() inside an async function with try/catch
that logs failures, or chain this.validateAvailability().catch(err => /* log
error with context */) so any DB/save errors are logged; ensure the log includes
context like "Presence.validateAvailability" and the error object.
In `@packages/agenda/src/Agenda.ts`:
- Line 473: The update path currently calls this._processDbResult(job, result)
with void which swallows its promise and can create unhandled rejections; change
that call to return the promise instead so its rejection is propagated
consistent with the other save paths that return this._processDbResult (see
_saveSingleJob, _saveUniqueJob, _saveNewJob and the call site result && void
this._processDbResult(job, result)). Ensure the method containing that line
returns the result of this._processDbResult(job, result) when result is truthy.
In `@packages/core-services/src/LocalBroker.ts`:
- Line 97: The call to instance.created() is awaited with void and so any
rejection is swallowed; change it to handle errors by attaching a .catch that
logs the error via the broker/logger and prevents leaving a half-initialized
service registered — e.g., call instance.created().catch(err => {
logger.error('service created failed', { service: instance.name, err }); /*
optionally remove from this.services or avoid calling registerService */ }); and
ensure the code that registers the service (the registerService or
this.services.set path) only runs on successful creation.
In `@packages/ddp-client/.eslintrc.json`:
- Around line 26-37: The override currently sets "parserOptions": { "project":
null } which disables all type-aware ESLint rules (not just the two promise
rules) — remove the parserOptions block (or set "project": true) from the
override so the base `@rocket.chat/eslint-config`'s type-aware override can apply,
and keep only the explicit rule disables for
"@typescript-eslint/no-misused-promises" and
"@typescript-eslint/no-floating-promises" under "rules".
In `@packages/instance-status/src/index.ts`:
- Around line 58-59: registerInstance currently fire-and-forgets createIndexes
by calling void createIndexes(), which can start registration/heartbeat before
the TTL index exists and can silence index creation failures; change this to
await createIndexes() inside registerInstance and handle errors (catch and log
or rethrow) so index creation failures surface and prevent starting the
heartbeat/registration flow until indexes are guaranteed; reference the
registerInstance function and the createIndexes function and ensure any
heartbeat/startHeartbeat logic only runs after the awaited createIndexes
completes successfully.
In `@packages/ui-voip/.eslintrc.json`:
- Around line 4-15: The override in .eslintrc.json currently sets
"parserOptions.project": null and turns off
"@typescript-eslint/no-misused-promises" and
"@typescript-eslint/no-floating-promises" for all "**/*.ts" and "**/*.tsx",
which weakens TypeScript linting; change this by restoring type-aware checks and
narrowing or softening the rules: set "parserOptions.project" to the appropriate
tsconfig (or remove the override so type-aware rules apply), and instead of
"off" change "@typescript-eslint/no-misused-promises" and
"@typescript-eslint/no-floating-promises" to "warn" or move them into a
file-scoped override for only the problematic files; update the override’s
"files" glob to target the specific exceptions rather than all TS/TSX files so
the rest of the repo inherits the standardized rules.
🧹 Nitpick comments (5)
apps/meteor/.scripts/run-ha.ts (1)
87-91:voidis correct here for the lint rule, but note these functions aren't actually async.
runMainandrunInstanceare declaredasyncyet contain noawait— they just call synchronousspawn(). Thevoidprefix satisfiesno-floating-promises, but if someone later adds actual async work to these functions, rejections will be silently swallowed. For a dev-only script this is low risk, but removing the unnecessaryasynckeyword from both functions would be a cleaner fix.packages/ddp-client/src/Connection.ts (1)
200-202: Consider adding.catchto prevent unhandled rejection on reconnect failure.
reconnect()can reject (e.g., Line 113: "Connection in progress"). Withvoid, that rejection is unhandled. A defensive.catchkeeps the retry loop clean:- void this.reconnect(); + void this.reconnect().catch(() => undefined);ee/packages/media-calls/src/internal/agents/CallSignalProcessor.ts (1)
266-270: Unhandled rejection risk ononDTMF.If
onDTMFrejects, the promise is unhandled. Since this is intentionally fire-and-forget, add a.catchto suppress:- void this.agent.oppositeAgent?.onDTMF(this.call._id, dtmf, duration || 2000); + void this.agent.oppositeAgent?.onDTMF(this.call._id, dtmf, duration || 2000).catch((err) => { + logger.error({ msg: 'Failed to send DTMF to opposite agent', err }); + });packages/ddp-client/src/DDPSDK.ts (1)
115-122:voidon reconnect login is acceptable, but a.catchwould improve debuggability.The
connectedevent handler can't propagate a promise, sovoidis appropriate. However, a silent token-login failure during reconnect could be hard to diagnose. Consider adding a.catchfor logging if a logger is available in this context.packages/i18n/.eslintrc.json (1)
8-17:"project": nulldisables all type-aware rules, making the explicit rule overrides redundant.Setting
parserOptions.projecttonullpreventstypescript-eslintfrom performing type-checked linting entirely. The two rule overrides on lines 15–16 are therefore redundant since those rules require type information to function. More importantly, this also silently disables every other type-aware rule (e.g.,no-unnecessary-type-assertion,no-unsafe-*,await-thenable, etc.).If the intent is only to disable the two promise rules, consider pointing
projectto the package'stsconfig.jsonand keeping just the rule overrides. If the intent is to skip type-aware linting in this package entirely, the explicit rule lines are unnecessary noise.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (35)
apps/meteor/.eslintrc.jsonapps/meteor/.scripts/run-ha.tsee/apps/account-service/src/service.tsee/apps/authorization-service/src/service.tsee/apps/ddp-streamer/src/Client.tsee/apps/ddp-streamer/src/DDPStreamer.tsee/apps/ddp-streamer/src/service.tsee/apps/omnichannel-transcript/src/service.tsee/apps/presence-service/src/service.tsee/apps/queue-worker/src/service.tsee/packages/federation-matrix/.eslintrc.jsonee/packages/media-calls/src/internal/agents/CallSignalProcessor.tsee/packages/media-calls/src/sip/Session.tsee/packages/media-calls/src/sip/providers/IncomingSipCall.tsee/packages/media-calls/src/sip/providers/OutgoingSipCall.tsee/packages/omnichannel-services/src/OmnichannelTranscript.tsee/packages/presence/src/Presence.tsee/packages/ui-theming/src/hooks/useThemeMode.tspackages/agenda/src/Agenda.tspackages/core-services/src/LocalBroker.tspackages/ddp-client/.eslintrc.jsonpackages/ddp-client/src/ClientStream.tspackages/ddp-client/src/Connection.tspackages/ddp-client/src/DDPSDK.tspackages/eslint-config/standard/index.jspackages/fuselage-ui-kit/.eslintrc.jsonpackages/i18n/.eslintrc.jsonpackages/instance-status/src/index.tspackages/livechat/.eslintrc.jsonpackages/media-signaling/src/lib/Call.tspackages/mock-providers/src/MockedAppRootBuilder.tsxpackages/ui-client/.eslintrc.jsonpackages/ui-contexts/.eslintrc.jsonpackages/ui-kit/.eslintrc.jsonpackages/ui-voip/.eslintrc.json
🧰 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/packages/media-calls/src/sip/providers/OutgoingSipCall.tsee/apps/ddp-streamer/src/service.tspackages/mock-providers/src/MockedAppRootBuilder.tsxee/packages/presence/src/Presence.tspackages/instance-status/src/index.tspackages/media-signaling/src/lib/Call.tsee/packages/ui-theming/src/hooks/useThemeMode.tspackages/agenda/src/Agenda.tsee/packages/media-calls/src/internal/agents/CallSignalProcessor.tsee/apps/omnichannel-transcript/src/service.tsee/apps/authorization-service/src/service.tspackages/ddp-client/src/Connection.tsee/apps/presence-service/src/service.tsee/apps/queue-worker/src/service.tsee/packages/media-calls/src/sip/providers/IncomingSipCall.tsee/apps/account-service/src/service.tspackages/core-services/src/LocalBroker.tsee/apps/ddp-streamer/src/Client.tspackages/eslint-config/standard/index.jsee/apps/ddp-streamer/src/DDPStreamer.tspackages/ddp-client/src/DDPSDK.tspackages/ddp-client/src/ClientStream.tsee/packages/media-calls/src/sip/Session.tsee/packages/omnichannel-services/src/OmnichannelTranscript.ts
🧠 Learnings (18)
📓 Common learnings
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 **/*.spec.ts : Use `.spec.ts` extension for test files (e.g., `login.spec.ts`)
Applied to files:
packages/fuselage-ui-kit/.eslintrc.jsonpackages/i18n/.eslintrc.jsonpackages/ui-contexts/.eslintrc.jsonpackages/ui-voip/.eslintrc.jsonpackages/ui-kit/.eslintrc.jsonee/packages/federation-matrix/.eslintrc.jsonpackages/ui-client/.eslintrc.jsonpackages/livechat/.eslintrc.jsonpackages/ddp-client/.eslintrc.jsonpackages/eslint-config/standard/index.js
📚 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 **/*.{ts,tsx,js} : Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Applied to files:
packages/fuselage-ui-kit/.eslintrc.jsonpackages/i18n/.eslintrc.jsonpackages/ui-contexts/.eslintrc.jsonpackages/ui-voip/.eslintrc.jsonpackages/ui-kit/.eslintrc.jsonee/packages/federation-matrix/.eslintrc.jsonapps/meteor/.eslintrc.jsonpackages/ui-client/.eslintrc.jsonpackages/livechat/.eslintrc.jsonpackages/ddp-client/.eslintrc.jsonpackages/eslint-config/standard/index.js
📚 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 **/*.{ts,tsx,js} : Avoid code comments in the implementation
Applied to files:
packages/fuselage-ui-kit/.eslintrc.jsonpackages/i18n/.eslintrc.jsonpackages/ui-contexts/.eslintrc.jsonpackages/ui-voip/.eslintrc.jsonpackages/ui-kit/.eslintrc.jsonee/packages/federation-matrix/.eslintrc.jsonapps/meteor/.eslintrc.jsonpackages/ui-client/.eslintrc.jsonpackages/ddp-client/.eslintrc.jsonpackages/eslint-config/standard/index.js
📚 Learning: 2025-12-10T21:00:54.909Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37091
File: ee/packages/abac/jest.config.ts:4-7
Timestamp: 2025-12-10T21:00:54.909Z
Learning: Rocket.Chat monorepo: Jest testMatch pattern '<rootDir>/src/**/*.spec.(ts|js|mjs)' is valid in this repo and used across multiple packages (e.g., packages/tools, ee/packages/omnichannel-services). Do not flag it as invalid in future reviews.
Applied to files:
packages/fuselage-ui-kit/.eslintrc.jsonpackages/i18n/.eslintrc.jsonpackages/ui-contexts/.eslintrc.jsonpackages/ui-voip/.eslintrc.jsonpackages/ui-kit/.eslintrc.jsonee/packages/federation-matrix/.eslintrc.jsonapps/meteor/.eslintrc.jsonpackages/ui-client/.eslintrc.jsonpackages/livechat/.eslintrc.jsonpackages/ddp-client/.eslintrc.jsonpackages/eslint-config/standard/index.js
📚 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:
packages/fuselage-ui-kit/.eslintrc.jsonpackages/i18n/.eslintrc.jsonpackages/ui-contexts/.eslintrc.jsonpackages/ui-voip/.eslintrc.jsonpackages/ui-kit/.eslintrc.jsonee/packages/federation-matrix/.eslintrc.jsonapps/meteor/.eslintrc.jsonpackages/ui-client/.eslintrc.jsonpackages/ddp-client/.eslintrc.jsonpackages/eslint-config/standard/index.js
📚 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:
packages/fuselage-ui-kit/.eslintrc.jsonpackages/i18n/.eslintrc.jsonpackages/ui-contexts/.eslintrc.jsonpackages/ui-voip/.eslintrc.jsonpackages/ui-kit/.eslintrc.jsonee/packages/federation-matrix/.eslintrc.jsonpackages/ui-client/.eslintrc.jsonpackages/ddp-client/.eslintrc.jsonpackages/eslint-config/standard/index.js
📚 Learning: 2025-11-05T21:04:35.787Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37357
File: ee/packages/federation-matrix/src/setup.ts:103-120
Timestamp: 2025-11-05T21:04:35.787Z
Learning: In Rocket.Chat's federation-matrix setup (ee/packages/federation-matrix/src/setup.ts and apps/meteor/ee/server/startup/federation.ts), configureFederationMatrixSettings does not need to be called before setupFederationMatrix. The SDK's init() establishes infrastructure (database, event handlers, APIs) first, and the configuration can be applied later via settings watchers before actual federation events are processed. The config only matters when events actually occur, at which point all infrastructure is already configured.
Applied to files:
ee/apps/ddp-streamer/src/service.ts
📚 Learning: 2025-10-06T20:30:45.540Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37152
File: packages/apps-engine/tests/test-data/storage/storage.ts:101-122
Timestamp: 2025-10-06T20:30:45.540Z
Learning: In `packages/apps-engine/tests/test-data/storage/storage.ts`, the stub methods (updatePartialAndReturnDocument, updateStatus, updateSetting, updateAppInfo, updateMarketplaceInfo) intentionally throw "Method not implemented." Tests using these methods must stub them using `SpyOn` from the test library rather than relying on actual implementations.
Applied to files:
packages/mock-providers/src/MockedAppRootBuilder.tsx
📚 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/.scripts/run-ha.tsapps/meteor/.eslintrc.json
📚 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:
ee/packages/federation-matrix/.eslintrc.json
📚 Learning: 2026-01-17T01:51:47.764Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38219
File: packages/core-typings/src/cloud/Announcement.ts:5-6
Timestamp: 2026-01-17T01:51:47.764Z
Learning: In packages/core-typings/src/cloud/Announcement.ts, the AnnouncementSchema.createdBy field intentionally overrides IBannerSchema.createdBy (object with _id and optional username) with a string enum ['cloud', 'system'] to match existing runtime behavior. This is documented as technical debt with a FIXME comment at apps/meteor/app/cloud/server/functions/syncWorkspace/handleCommsSync.ts:53 and should not be flagged as an error until the runtime behavior is corrected.
Applied to files:
packages/agenda/src/Agenda.tsapps/meteor/.eslintrc.jsonee/apps/ddp-streamer/src/DDPStreamer.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/.eslintrc.json
📚 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/.eslintrc.jsonpackages/eslint-config/standard/index.js
📚 Learning: 2025-10-28T19:39:58.182Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37328
File: packages/ui-voip/src/v2/useTonePlayer.ts:29-30
Timestamp: 2025-10-28T19:39:58.182Z
Learning: In packages/ui-voip/src/v2/useTonePlayer.ts, the DTMF tones generated by TonePlayer are for local auditory feedback to the user only. The actual DTMF signals are transmitted separately through the WebRTC channel. Therefore, filter configurations should prioritize user comfort and audio quality over technical DTMF frequency accuracy.
Applied to files:
ee/packages/media-calls/src/internal/agents/CallSignalProcessor.ts
📚 Learning: 2025-09-15T21:34:39.812Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 36717
File: packages/ui-voip/src/providers/useCallSounds.ts:6-21
Timestamp: 2025-09-15T21:34:39.812Z
Learning: The voipSounds methods (playDialer, playRinger, playCallEnded) from useCustomSound return proper offCallbackHandler cleanup functions, not void as some type definitions might suggest.
Applied to files:
ee/packages/media-calls/src/internal/agents/CallSignalProcessor.ts
📚 Learning: 2025-11-19T18:20:37.116Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37419
File: apps/meteor/server/services/media-call/service.ts:141-141
Timestamp: 2025-11-19T18:20:37.116Z
Learning: In apps/meteor/server/services/media-call/service.ts, the sendHistoryMessage method should use call.caller.id or call.createdBy?.id as the message author, not call.transferredBy?.id. Even for transferred calls, the message should appear in the DM between the two users who are calling each other, not sent by the person who transferred the call.
Applied to files:
ee/packages/media-calls/src/internal/agents/CallSignalProcessor.ts
📚 Learning: 2026-01-27T20:57:56.529Z
Learnt from: nazabucciarelli
Repo: RocketChat/Rocket.Chat PR: 38294
File: apps/meteor/server/hooks/sauMonitorHooks.ts:0-0
Timestamp: 2026-01-27T20:57:56.529Z
Learning: In Rocket.Chat, the `accounts.login` event listened to by DeviceManagementService is only broadcast when running in microservices mode (via DDPStreamer), not in monolith mode. The `Accounts.onLogin` hook in sauMonitorHooks.ts runs in monolith deployments. These are mutually exclusive deployment modes, so there's no duplication of event emissions between these two code paths.
Applied to files:
ee/apps/ddp-streamer/src/DDPStreamer.tspackages/ddp-client/src/DDPSDK.ts
🧬 Code graph analysis (2)
packages/mock-providers/src/MockedAppRootBuilder.tsx (1)
apps/meteor/app/lib/server/functions/notifications/email.js (1)
lng(25-25)
packages/instance-status/src/index.ts (1)
ee/packages/omnichannel-services/src/QueueWorker.ts (1)
createIndexes(54-63)
⏰ 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 comments (24)
ee/packages/media-calls/src/sip/Session.ts (1)
149-164: LGTM —voidprefix is appropriate here given the event-based error handling.The
connectanderrorevent handlers registered ininitializeDrachtio()(lines 124–133) already handle connection outcomes, so discarding the promise withvoidto satisfyno-floating-promisesis the right approach.apps/meteor/.scripts/run-ha.ts (1)
98-98: LGTM — standard pattern for top-level async invocation.Using
voidfor the top-level entry point is the conventional way to satisfyno-floating-promisesin scripts.packages/mock-providers/src/MockedAppRootBuilder.tsx (3)
121-129: LGTM — synchronous throws align with existingcallEndpointpattern.These stubs now throw synchronously instead of returning rejected promises. This matches the pattern already used by
callEndpoint(Line 116). SinceuploadToEndpointandcallMethod(Lines 119–120) still usePromise.reject, the file now has two stub-failure styles, but this is acceptable for mock/test infrastructure.
650-661: LGTM —voidoperator correctly silences floating-promise lint warnings.
730-730: LGTM — fire-and-forgeti18n.init()properly annotated withvoid.packages/eslint-config/standard/index.js (2)
129-165: Overall approach looks good — type-aware promise rules in a dedicated override is the right pattern.The exclusion list is thorough for test files and build configs, and
checksVoidReturn.arguments: falseis a pragmatic choice to reduce noise from callback-style APIs.
130-130: The inconsistency is confirmed:.ctsand.mtsfiles exist in the repository but are excluded from the second override.The repo contains 1
.ctsfile (packages/i18n/src/index.cts) and 5.mtsfiles (build utilities inpackages/i18n/src/scripts/), neither of which match any of the second override'sexcludedFilespatterns. Confirm whether this exclusion is intentional—for example, if build scripts and CTS wrappers are intentionally treated differently from application TypeScript code.packages/media-signaling/src/lib/Call.ts (1)
833-833: This pattern is intentional—errors are handled through the event emitter, not promise rejection.The
voidusage here aligns with the codebase's error-handling architecture.addNegotiationdoesn't throw or reject; instead, errors are propagated via theNegotiationManager.emitterand caught inCall.onNegotiationError, which sends them back to the remote peer. This is consistent with similar code inNegotiationManager.setRemoteDescription, which includes a comment explaining this design: "No need to handle errors here as they are already handled by the 'error' event." Usingawaitwould not capture errors any differently and could disrupt the async queueing behavior inprocessNegotiations.ee/apps/ddp-streamer/src/service.ts (1)
9-9: LGTM — theunhandledRejectionhandler (Line 45) provides a safety net for bootstrap failures.packages/core-services/src/LocalBroker.ts (1)
127-127: Fire-and-forget onbroadcastLocalis acceptable here.
broadcastLocalonly calls the synchronousthis.events.emit(...), so the promise resolves immediately and there's no meaningful rejection to handle.apps/meteor/.eslintrc.json (1)
142-148: Scoping the promise rule relaxation to client-only code is reasonable.Disabling
no-misused-promisesandno-floating-promisesonly forclient/**andee/client/**is a sensible choice — React event handlers and UI code commonly produce fire-and-forget patterns. Server-side code retains the stricter linting via the broader TS override above.packages/ddp-client/src/ClientStream.ts (1)
122-125:voidonunsubscribemakes the existing fire-and-forget intent explicit.Since
stop()is synchronous and callers can't await it, thevoidprefix correctly signals intent. The previous code also didn't handle the promise — this just satisfies the linter.ee/apps/ddp-streamer/src/Client.ts (1)
168-168: LGTM —voidcorrectly signals fire-and-forget intent.Both
callMethodandcallSubscribealready chain errors internally via.catch(), so thevoidprefix accurately communicates that the returned promise is intentionally discarded.Also applies to: 177-177
ee/apps/authorization-service/src/service.ts (1)
10-10:voidmakes the already-floating IIFE explicit — fine as a lint fix.Note that a rejection inside this IIFE (e.g.,
getConnection()failing) will still surface as an unhandled promise rejection and crash the process, which is likely the desired behavior for a service entry point. No behavioral change here.ee/packages/media-calls/src/sip/providers/OutgoingSipCall.ts (1)
242-242:voidmakes the existing fire-and-forget explicit, but note that rejections bypass the surroundingtry/catch.Since the promise is not awaited, any rejection from
onRemoteDescriptionChangedwill surface as an unhandled promise rejection rather than being caught by thetry/catchon line 223. This is pre-existing behavior (unchanged by this PR), but worth noting ifonRemoteDescriptionChangedcan fail in practice.packages/livechat/.eslintrc.json (1)
79-104: Settingproject: nullmakes the explicit rule disables redundant.Both
no-misused-promisesandno-floating-promisesare type-aware rules that requireparserOptions.projectto function. Withproject: null, they are already inoperative. The explicit"off"entries are harmless but purely documentary.This is consistent across all packages in this PR, so presumably intentional for clarity.
packages/ui-client/.eslintrc.json (1)
4-15: Consistent with the repo-wide pattern.ee/packages/federation-matrix/.eslintrc.json (1)
4-15: Consistent with the repo-wide pattern.ee/packages/presence/src/Presence.ts (1)
294-305: Fire-and-forget broadcast is appropriate here.The
broadcastmethod is already typed as returningvoid, and the optional chaining onthis.api?.broadcastmeans it gracefully no-ops whenapiis unavailable. Thevoidprefix correctly signals intent.ee/apps/ddp-streamer/src/DDPStreamer.ts (1)
261-261: Fire-and-forget onregisterInstanceis acceptable here.The
ping()mechanism (line 92–98) re-registers the instance if the heartbeat finds no matching document, providing a self-healing fallback. Silent failure on initial registration is tolerable.ee/packages/ui-theming/src/hooks/useThemeMode.ts (1)
21-21: Newhigh-contrasttheme mode entry looks correct.Follows the existing pattern and the
useThemefunction (line 31) correctly handles the'high-contrast'case before falling through to'light'.packages/agenda/src/Agenda.ts (3)
146-150: LGTM — constructor fire-and-forget is consistent with the_readyevent pattern.The
_readypromise (line 144) synchronizes on the'ready'event emitted bydbInit, so thesevoidcalls in the constructor are correct.
833-837: LGTM — fire-and-forget with a complete error-handling chain.The
.catch()ensures the promise never rejects unhandled, and results flow into_processJobResult.
867-867: LGTM —voidin synchronous methods correctly signals intentional fire-and-forget.
_jobProcessingandprocessJobsare synchronous; thevoidprefix satisfies the@typescript-eslint/no-floating-promisesrule while preserving the intended behavior.Also applies to: 922-922, 929-929
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
6750bb5 to
8b62d88
Compare
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.
No issues found across 75 files
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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ee/apps/authorization-service/src/service.ts (1)
10-44:⚠️ Potential issue | 🟠 MajorNo unhandled-rejection safety net for the bootstrap IIFE.
Same concern as
omnichannel-transcript/src/service.ts: this file has noprocess.on('unhandledRejection', ...)handler. IfgetConnection(),api.start(), or any other awaited call rejects, the error is silently lost. Consider adding a.catch()on the IIFE or a global rejection handler, asddp-streamer/src/service.tsdoes.Suggested fix
-void (async () => { +void (async () => { // ... -})(); +})().catch((err) => { + console.error('Failed to start authorization-service', err); + process.exit(1); +});
🤖 Fix all issues with AI agents
In `@ee/apps/ddp-streamer/src/DDPStreamer.ts`:
- Line 261: The call to InstanceStatus.registerInstance('ddp-streamer', {}) is
prefixed with void which prevents its rejection from being caught by the
surrounding try/catch; change this to await
InstanceStatus.registerInstance('ddp-streamer', {}) so registration failures
propagate into the existing error handler (or, if the enclosing function is not
async, make it async or explicitly handle the promise rejection and rethrow).
Ensure the enclosing startup function (the one containing the try/catch around
the register call) is async or otherwise awaits the promise so service
registration failures are surfaced and handled.
In `@ee/packages/media-calls/src/sip/providers/OutgoingSipCall.ts`:
- Line 242: The fire-and-forget call to
callerAgent.onRemoteDescriptionChanged(this.call._id, negotiationId) risks
unhandled rejections and skips the existing catch cleanup (sending SIP error and
removing the negotiation from this.inboundRenegotiations); change the call to
either await callerAgent.onRemoteDescriptionChanged(...) inside the surrounding
try so rejections hit the catch, or if keeping non-blocking, append a .catch(err
=> { send the same SIP error response (res) and remove
this.inboundRenegotiations.get(negotiationId) / delete the entry from
this.inboundRenegotiations }) so any failure performs the same cleanup as the
current catch block rather than leaving a stale entry.
In `@packages/livechat/src/lib/hooks.ts`:
- Line 94: The call to createOrUpdateGuest from updateIframeGuestData currently
discards promise rejections (void createOrUpdateGuest(guestData)); attach a
.catch handler to that returned promise to log failures so network errors from
Livechat.grantVisitor are visible; locate the call site in updateIframeGuestData
and change it to call createOrUpdateGuest(guestData).catch(err =>
console.error('createOrUpdateGuest failed', err)) (or use the existing
widget/logger if available) to surface errors.
🧹 Nitpick comments (2)
packages/livechat/src/entry.ts (1)
18-18: Consider adding a.catch()to surface initialization failures.With
void init(), if the dynamic import or render fails, the rejection is silently swallowed and the user sees a blank page with no feedback. A minimal error handler would improve debuggability:Suggested change
-void init(); +void init().catch((err) => console.error('Failed to initialize app:', err));packages/livechat/src/lib/connection.ts (1)
54-64:clearAlertsanddisplayAlertno longer need to beasync.With the
awaitremoved from thestore.setStatecalls, these methods contain no asynchronous operations. Theasynckeyword is now misleading — it wraps the return in an unnecessaryPromise. Note that callers inhandleConnectedandhandleDisconnectedstillawaitthese methods, so removingasync(and updating return types) would be a clean follow-up.Suggested change
- async clearAlerts() { + clearAlerts() { const { alerts } = store.state; store.setState({ alerts: alerts?.filter((alert) => ![livechatDisconnectedAlertId, livechatConnectedAlertId].includes(alert.id)), }); }, - async displayAlert(alert = {}) { + displayAlert(alert = {}) { const { alerts } = store.state; store.setState({ alerts: (alerts?.push(alert), alerts) }); },
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (54)
apps/meteor/.eslintrc.jsonapps/meteor/.scripts/run-ha.tsee/apps/account-service/src/service.tsee/apps/authorization-service/src/service.tsee/apps/ddp-streamer/src/Client.tsee/apps/ddp-streamer/src/DDPStreamer.tsee/apps/ddp-streamer/src/service.tsee/apps/omnichannel-transcript/src/service.tsee/apps/presence-service/src/service.tsee/apps/queue-worker/src/service.tsee/packages/federation-matrix/src/events/index.tsee/packages/federation-matrix/tsconfig.build.jsonee/packages/federation-matrix/tsconfig.jsonee/packages/media-calls/src/internal/agents/CallSignalProcessor.tsee/packages/media-calls/src/sip/Session.tsee/packages/media-calls/src/sip/providers/IncomingSipCall.tsee/packages/media-calls/src/sip/providers/OutgoingSipCall.tsee/packages/omnichannel-services/src/OmnichannelTranscript.tsee/packages/presence/src/Presence.tsee/packages/ui-theming/src/hooks/useThemeMode.tspackages/agenda/src/Agenda.tspackages/core-services/src/LocalBroker.tspackages/ddp-client/__examples__/simple.tspackages/ddp-client/package.jsonpackages/ddp-client/src/ClientStream.tspackages/ddp-client/src/Connection.tspackages/ddp-client/src/DDPSDK.tspackages/ddp-client/tsconfig.build.jsonpackages/ddp-client/tsconfig.jsonpackages/eslint-config/standard/index.jspackages/fuselage-ui-kit/.eslintrc.jsonpackages/fuselage-ui-kit/src/blocks/VideoConferenceBlock/VideoConferenceBlock.tsxpackages/fuselage-ui-kit/src/elements/ButtonElement.tsxpackages/fuselage-ui-kit/src/elements/ChannelsSelectElement/ChannelsSelectElement.tsxpackages/fuselage-ui-kit/src/elements/ChannelsSelectElement/MultiChannelsSelectElement.tsxpackages/fuselage-ui-kit/src/elements/IconButtonElement.tsxpackages/fuselage-ui-kit/src/elements/MultiStaticSelectElement.tsxpackages/fuselage-ui-kit/src/elements/OverflowElement.tsxpackages/fuselage-ui-kit/src/elements/StaticSelectElement.tsxpackages/fuselage-ui-kit/src/elements/TabElement.tsxpackages/fuselage-ui-kit/src/elements/UsersSelectElement/MultiUsersSelectElement.tsxpackages/fuselage-ui-kit/src/elements/UsersSelectElement/UsersSelectElement.tsxpackages/i18n/tsconfig.jsonpackages/instance-status/src/index.tspackages/livechat/.eslintrc.jsonpackages/livechat/src/components/App/App.tsxpackages/livechat/src/entry.tspackages/livechat/src/hooks/livechatRoomSubscriptionHooks.tspackages/livechat/src/hooks/useRoomMessagesSubscription.tspackages/livechat/src/lib/connection.tspackages/livechat/src/lib/hooks.tspackages/livechat/src/lib/transcript.tspackages/media-signaling/src/lib/Call.tspackages/mock-providers/src/MockedAppRootBuilder.tsx
✅ Files skipped from review due to trivial changes (1)
- packages/livechat/src/components/App/App.tsx
🚧 Files skipped from review as they are similar to previous changes (14)
- packages/core-services/src/LocalBroker.ts
- packages/ddp-client/src/ClientStream.ts
- ee/apps/ddp-streamer/src/Client.ts
- packages/media-signaling/src/lib/Call.ts
- packages/instance-status/src/index.ts
- apps/meteor/.eslintrc.json
- ee/apps/account-service/src/service.ts
- ee/packages/media-calls/src/sip/providers/IncomingSipCall.ts
- apps/meteor/.scripts/run-ha.ts
- packages/eslint-config/standard/index.js
- ee/packages/media-calls/src/sip/Session.ts
- ee/packages/media-calls/src/internal/agents/CallSignalProcessor.ts
- packages/ddp-client/src/Connection.ts
- packages/agenda/src/Agenda.ts
🧰 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:
packages/livechat/src/lib/transcript.tspackages/fuselage-ui-kit/src/elements/UsersSelectElement/UsersSelectElement.tsxpackages/fuselage-ui-kit/src/blocks/VideoConferenceBlock/VideoConferenceBlock.tsxpackages/ddp-client/src/DDPSDK.tspackages/fuselage-ui-kit/src/elements/ChannelsSelectElement/MultiChannelsSelectElement.tsxee/apps/queue-worker/src/service.tspackages/fuselage-ui-kit/src/elements/ButtonElement.tsxpackages/livechat/src/hooks/livechatRoomSubscriptionHooks.tsee/apps/authorization-service/src/service.tspackages/livechat/src/hooks/useRoomMessagesSubscription.tsee/apps/ddp-streamer/src/service.tsee/packages/presence/src/Presence.tsee/apps/ddp-streamer/src/DDPStreamer.tsee/packages/media-calls/src/sip/providers/OutgoingSipCall.tspackages/fuselage-ui-kit/src/elements/UsersSelectElement/MultiUsersSelectElement.tsxpackages/fuselage-ui-kit/src/elements/MultiStaticSelectElement.tsxpackages/fuselage-ui-kit/src/elements/StaticSelectElement.tsxpackages/fuselage-ui-kit/src/elements/TabElement.tsxee/packages/federation-matrix/src/events/index.tspackages/ddp-client/__examples__/simple.tspackages/fuselage-ui-kit/src/elements/OverflowElement.tsxpackages/mock-providers/src/MockedAppRootBuilder.tsxpackages/fuselage-ui-kit/src/elements/IconButtonElement.tsxpackages/livechat/src/lib/connection.tsee/apps/omnichannel-transcript/src/service.tspackages/livechat/src/entry.tsee/packages/omnichannel-services/src/OmnichannelTranscript.tspackages/fuselage-ui-kit/src/elements/ChannelsSelectElement/ChannelsSelectElement.tsxpackages/livechat/src/lib/hooks.tsee/packages/ui-theming/src/hooks/useThemeMode.tsee/apps/presence-service/src/service.ts
🧠 Learnings (16)
📚 Learning: 2025-12-18T15:18:31.688Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37773
File: apps/meteor/client/views/mediaCallHistory/MediaCallHistoryInternal.tsx:24-34
Timestamp: 2025-12-18T15:18:31.688Z
Learning: In apps/meteor/client/views/mediaCallHistory/MediaCallHistoryInternal.tsx, for internal call history items, the item.contactId is guaranteed to always match either the caller.id or callee.id in the call data, so the contact resolution in getContact will never result in undefined.
Applied to files:
packages/fuselage-ui-kit/src/blocks/VideoConferenceBlock/VideoConferenceBlock.tsx
📚 Learning: 2026-01-27T20:57:56.529Z
Learnt from: nazabucciarelli
Repo: RocketChat/Rocket.Chat PR: 38294
File: apps/meteor/server/hooks/sauMonitorHooks.ts:0-0
Timestamp: 2026-01-27T20:57:56.529Z
Learning: In Rocket.Chat, the `accounts.login` event listened to by DeviceManagementService is only broadcast when running in microservices mode (via DDPStreamer), not in monolith mode. The `Accounts.onLogin` hook in sauMonitorHooks.ts runs in monolith deployments. These are mutually exclusive deployment modes, so there's no duplication of event emissions between these two code paths.
Applied to files:
packages/ddp-client/src/DDPSDK.tsee/apps/ddp-streamer/src/DDPStreamer.ts
📚 Learning: 2025-11-17T15:07:13.273Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37398
File: packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx:357-363
Timestamp: 2025-11-17T15:07:13.273Z
Learning: In packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx, IconElement is a presentational, non-actionable element that does not require wrapping in AppIdProvider, similar to plain_text and mrkdwn renderers. Only actionable elements (those with actions, actionId, or interactive behavior) should be wrapped in AppIdProvider.
Applied to files:
packages/fuselage-ui-kit/src/elements/ButtonElement.tsxpackages/fuselage-ui-kit/src/elements/StaticSelectElement.tsxpackages/fuselage-ui-kit/src/elements/TabElement.tsxpackages/fuselage-ui-kit/src/elements/IconButtonElement.tsx
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings by mapping subscription documents to room IDs, never undefined, even when user has no room subscriptions.
Applied to files:
packages/livechat/src/hooks/useRoomMessagesSubscription.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings (mapping subscription documents to room IDs), never undefined, even when user has no room subscriptions.
Applied to files:
packages/livechat/src/hooks/useRoomMessagesSubscription.ts
📚 Learning: 2025-11-05T21:04:35.787Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37357
File: ee/packages/federation-matrix/src/setup.ts:103-120
Timestamp: 2025-11-05T21:04:35.787Z
Learning: In Rocket.Chat's federation-matrix setup (ee/packages/federation-matrix/src/setup.ts and apps/meteor/ee/server/startup/federation.ts), configureFederationMatrixSettings does not need to be called before setupFederationMatrix. The SDK's init() establishes infrastructure (database, event handlers, APIs) first, and the configuration can be applied later via settings watchers before actual federation events are processed. The config only matters when events actually occur, at which point all infrastructure is already configured.
Applied to files:
ee/apps/ddp-streamer/src/service.tsee/packages/federation-matrix/src/events/index.ts
📚 Learning: 2025-12-10T21:00:54.909Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37091
File: ee/packages/abac/jest.config.ts:4-7
Timestamp: 2025-12-10T21:00:54.909Z
Learning: Rocket.Chat monorepo: Jest testMatch pattern '<rootDir>/src/**/*.spec.(ts|js|mjs)' is valid in this repo and used across multiple packages (e.g., packages/tools, ee/packages/omnichannel-services). Do not flag it as invalid in future reviews.
Applied to files:
packages/ddp-client/tsconfig.build.jsonpackages/fuselage-ui-kit/.eslintrc.jsonee/packages/federation-matrix/tsconfig.jsonpackages/ddp-client/tsconfig.jsonpackages/i18n/tsconfig.jsonee/packages/federation-matrix/tsconfig.build.jsonpackages/livechat/.eslintrc.json
📚 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 **/*.spec.ts : Use `.spec.ts` extension for test files (e.g., `login.spec.ts`)
Applied to files:
packages/fuselage-ui-kit/.eslintrc.jsonee/packages/federation-matrix/tsconfig.jsonpackages/ddp-client/tsconfig.jsonee/packages/federation-matrix/tsconfig.build.json
📚 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 **/*.{ts,tsx,js} : Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Applied to files:
packages/fuselage-ui-kit/.eslintrc.jsonee/packages/federation-matrix/tsconfig.jsonpackages/ddp-client/package.jsonpackages/ddp-client/tsconfig.jsonpackages/i18n/tsconfig.jsonee/packages/federation-matrix/tsconfig.build.jsonpackages/livechat/.eslintrc.json
📚 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 **/*.{ts,tsx,js} : Avoid code comments in the implementation
Applied to files:
packages/fuselage-ui-kit/.eslintrc.jsonpackages/livechat/.eslintrc.json
📚 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:
packages/fuselage-ui-kit/.eslintrc.jsonpackages/ddp-client/tsconfig.json
📚 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:
packages/fuselage-ui-kit/.eslintrc.json
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: UserBridge.doGetUserRoomIds in packages/apps-engine/src/server/bridges/UserBridge.ts has a bug where it implicitly returns undefined when the app lacks read permission (missing return statement in the else case of the permission check).
Applied to files:
ee/apps/ddp-streamer/src/DDPStreamer.ts
📚 Learning: 2025-12-09T20:01:00.324Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37532
File: ee/packages/federation-matrix/src/FederationMatrix.ts:920-927
Timestamp: 2025-12-09T20:01:00.324Z
Learning: When reviewing federation invite handling in Rocket.Chat (specifically under ee/packages/federation-matrix), understand that rejecting an invite via federationSDK.rejectInvite() triggers an event-driven cleanup: a leave event is emitted and handled by handleLeave() in ee/packages/federation-matrix/src/events/member.ts, which calls Room.performUserRemoval() to remove the subscription. Do not add explicit cleanup in the reject branch of handleInvite(); rely on the existing leave-event flow for cleanup. If making changes, ensure this invariant remains and that any related paths still funnel cleanup through the leave event to avoid duplicate or missing removals.
Applied to files:
ee/packages/federation-matrix/src/events/index.ts
📚 Learning: 2025-10-06T20:30:45.540Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37152
File: packages/apps-engine/tests/test-data/storage/storage.ts:101-122
Timestamp: 2025-10-06T20:30:45.540Z
Learning: In `packages/apps-engine/tests/test-data/storage/storage.ts`, the stub methods (updatePartialAndReturnDocument, updateStatus, updateSetting, updateAppInfo, updateMarketplaceInfo) intentionally throw "Method not implemented." Tests using these methods must stub them using `SpyOn` from the test library rather than relying on actual implementations.
Applied to files:
packages/mock-providers/src/MockedAppRootBuilder.tsx
📚 Learning: 2026-01-17T01:51:47.764Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38219
File: packages/core-typings/src/cloud/Announcement.ts:5-6
Timestamp: 2026-01-17T01:51:47.764Z
Learning: In packages/core-typings/src/cloud/Announcement.ts, the AnnouncementSchema.createdBy field intentionally overrides IBannerSchema.createdBy (object with _id and optional username) with a string enum ['cloud', 'system'] to match existing runtime behavior. This is documented as technical debt with a FIXME comment at apps/meteor/app/cloud/server/functions/syncWorkspace/handleCommsSync.ts:53 and should not be flagged as an error until the runtime behavior is corrected.
Applied to files:
ee/apps/omnichannel-transcript/src/service.tspackages/livechat/.eslintrc.json
🧬 Code graph analysis (6)
ee/apps/ddp-streamer/src/DDPStreamer.ts (1)
packages/instance-status/src/index.ts (1)
InstanceStatus(104-110)
packages/fuselage-ui-kit/src/elements/TabElement.tsx (1)
apps/meteor/app/integrations/server/lib/ScriptEngine.ts (1)
disabled(167-169)
ee/packages/federation-matrix/src/events/index.ts (5)
ee/packages/federation-matrix/src/events/ping.ts (1)
ping(3-7)ee/packages/federation-matrix/src/events/message.ts (1)
message(113-416)ee/packages/federation-matrix/src/events/reaction.ts (1)
reaction(9-92)ee/packages/federation-matrix/src/events/member.ts (1)
member(257-280)ee/packages/federation-matrix/src/events/edu.ts (1)
edus(9-74)
packages/mock-providers/src/MockedAppRootBuilder.tsx (1)
apps/meteor/app/lib/server/functions/notifications/email.js (1)
lng(25-25)
packages/livechat/src/lib/connection.ts (1)
packages/livechat/src/lib/room.js (1)
alert(89-89)
packages/livechat/src/lib/hooks.ts (1)
packages/core-services/src/index.ts (1)
api(55-55)
⏰ 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). (13)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: 🔨 Test UI (EE) / MongoDB 8.2 coverage (2/5)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 8.2 coverage (1/5)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 8.2 coverage (4/5)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 8.2 coverage (3/5)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 8.2 coverage (5/5)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 8.2 (2/4)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 8.2 (3/4)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 8.2 (1/4)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 8.2 (4/4)
- GitHub Check: 🔨 Test API (CE) / MongoDB 8.2 (1/1)
- GitHub Check: 🔨 Test API (EE) / MongoDB 8.2 coverage (1/1)
- GitHub Check: 🔨 Test Federation Matrix
🔇 Additional comments (36)
packages/ddp-client/__examples__/simple.ts (1)
37-39: LGTM!The
voidprefix correctly marks the async IIFE as intentionally fire-and-forget, satisfying the@typescript-eslint/no-floating-promisesrule without altering behavior.packages/livechat/src/hooks/useRoomMessagesSubscription.ts (1)
14-16: LGTM —voidprefix correctly marks the fire-and-forget promise.The
voidoperator makes the intentional discard of theonMessagepromise explicit for the linter without changing runtime behavior.packages/fuselage-ui-kit/src/elements/OverflowElement.tsx (1)
25-25: LGTM —voidprefixes correctly suppress floating promises in fire-and-forget UI handlers.Both changes follow the established PR-wide pattern for satisfying
@typescript-eslint/no-floating-promiseswithout altering control flow. The fire-and-forget semantics are appropriate for these UI event callbacks.Also applies to: 41-41
packages/fuselage-ui-kit/src/blocks/VideoConferenceBlock/VideoConferenceBlock.tsx (1)
51-88: LGTM —voidprefix correctly suppresses floating-promise lint errors.The three handlers (
joinHandler,callAgainHandler,openCallInfo) now usevoid action(...)to explicitly mark the returned promise as intentionally unhandled, which is the idiomatic pattern for satisfying@typescript-eslint/no-floating-promisesin fire-and-forget event handlers.ee/packages/ui-theming/src/hooks/useThemeMode.ts (1)
17-22: LGTM — type refinement and newhigh-contrastentry look correct.The internal type now accurately reflects the
saveUserPreferencesreturn type, while the external hook signature on line 11 intentionally narrows to() => voidfor fire-and-forget semantics. The new'high-contrast'entry is consistent with the existing pattern.packages/ddp-client/package.json (1)
10-11: LGTM!Clean separation:
buildanddevnow target the dedicatedtsconfig.build.json(which excludes tests and sets output dirs), whiletypecheckcontinues to use the broadertsconfig.json. This is a well-structured split.packages/ddp-client/tsconfig.json (1)
1-10: LGTM!Good refactoring —
rootDir/outDirmoved to the build config where they belong, andincludeexpanded to cover__examples__and__mocks__for broader type-checking coverage.Nit:
sourceMap: trueon line 7 is effectively unused since this config is only consumed bytsc --noEmit, but it's harmless.packages/ddp-client/tsconfig.build.json (1)
1-13: LGTM!Clean dedicated build config. Correctly scopes
includeto./src/**/*and excludes test files from the compiled output.packages/livechat/.eslintrc.json (1)
99-100: LGTM — consistent with the PR-wide pattern of disablingno-misused-promisesin UI packages.packages/fuselage-ui-kit/src/elements/ButtonElement.tsx (1)
14-16: LGTM — correct use ofvoidto suppress the floating promise in a synchronous event handler.packages/fuselage-ui-kit/src/elements/UsersSelectElement/UsersSelectElement.tsx (1)
26-31: LGTM — consistentvoidprefix for fire-and-forget action dispatch.packages/fuselage-ui-kit/src/elements/StaticSelectElement.tsx (1)
21-26: LGTM — standardvoidprefix for the action call.packages/fuselage-ui-kit/src/elements/UsersSelectElement/MultiUsersSelectElement.tsx (1)
22-27: LGTM — consistent with the single-select counterpart, correctly guarding for array values.packages/fuselage-ui-kit/.eslintrc.json (1)
3-11: LGTM — correctly disablesno-misused-promisesfor TypeScript files in this UI kit package.Minor note: this file uses
**/*.tsglob patterns whilepackages/livechat/.eslintrc.jsonuses*.ts. Both work in ESLint overrides, but you may want to standardize the glob style across packages for consistency.packages/fuselage-ui-kit/src/elements/ChannelsSelectElement/ChannelsSelectElement.tsx (1)
21-26: LGTM — consistentvoidprefix matching the pattern across all select elements.packages/fuselage-ui-kit/src/elements/ChannelsSelectElement/MultiChannelsSelectElement.tsx (1)
23-23: LGTM!The
voidprefix correctly marks the fire-and-forget intent for the promise-returningactioncall.packages/fuselage-ui-kit/src/elements/IconButtonElement.tsx (1)
16-18: LGTM!Consistent
void-prefixed fire-and-forget pattern for the click handler.packages/fuselage-ui-kit/src/elements/MultiStaticSelectElement.tsx (1)
22-27: LGTM!Consistent
void-prefixed fire-and-forget pattern for the change handler.packages/fuselage-ui-kit/src/elements/TabElement.tsx (1)
25-28: LGTM!The
voidprefix correctly discards the promise fromaction(e). Operator precedence is fine here —voidbinds tighter than&&, soaction(e)is called and its result discarded only when!disabled.packages/mock-providers/src/MockedAppRootBuilder.tsx (3)
121-129: Synchronous throws are consistent with these methods'voidreturn types.The change from
Promise.reject(...)to synchronousthrowaligns these stubs with their non-async signatures, whileuploadToEndpointandcallMethod(lines 119–120) correctly retainPromise.rejectsince they returnPromise<…>. Clean separation.
652-657:voidprefix correctly satisfiesno-floating-promises.Both the already-initialized and the
on('initialized', ...)callback paths properly mark thechangeLanguagereturn value as intentionally discarded.
730-730:void i18n.init()is appropriate here.Given
initImmediate: false(line 628), init completes synchronously, but the method still returns aPromiseat the type level. Thevoidoperator correctly signals the linter that the promise is intentionally unhandled.packages/livechat/src/lib/transcript.ts (1)
62-62: LGTM —voidprefix correctly marks the alert as fire-and-forget.The
transcriptSentAlertis a non-critical UI notification with a timeout, so ignoring its promise is reasonable here.packages/livechat/src/lib/connection.ts (1)
31-31: LGTM —voidis appropriate here since alert clearing after connect is non-critical.packages/i18n/tsconfig.json (2)
17-17: Good addition ofjest.config.tstoinclude.Ensures the Jest configuration file is type-checked alongside the source.
4-15:target: "es2024"withlib: ["ES2023"]creates a technical mismatch, but has no practical impact.The config specifies
target: "es2024"whilelibonly provides ES2023 type definitions. This is technically inconsistent — the compiler assumes an ES2024 runtime, but type definitions don't include ES2024-only APIs likeObject.groupBy,Promise.withResolvers, orArrayBuffer.resize.However, the code in this package doesn't use any ES2024-only APIs, so the mismatch has no real impact. If this inconsistency bothers you, either align
targetandlib(both ES2023) or expandlibto match the target, but it's not blocking any actual issues.packages/livechat/src/hooks/livechatRoomSubscriptionHooks.ts (2)
15-15: LGTM —voidprefix correctly marks fire-and-forget promises.Both additions properly silence
@typescript-eslint/no-floating-promisesfor these intentionally unhandled async calls.Also applies to: 45-45
28-31: No change needed —onAgentStatusChangeis correctly called withoutvoidprefix.
onAgentStatusChangeis a synchronous function and does not return a Promise, unlikeonAgentChange(async) andonQueuePositionChange(async). Thevoidprefix is only needed for functions that return Promises to satisfy ESLint's floating promises rule. The code is correct as-is.ee/apps/ddp-streamer/src/service.ts (1)
9-9: LGTM!The
voidprefix correctly satisfies theno-floating-promisesESLint rule, and this file already hasprocess.on('unhandledRejection', ...)andprocess.on('uncaughtException', ...)handlers (Lines 45–70) to catch bootstrap failures.packages/ddp-client/src/DDPSDK.ts (1)
115-122: LGTM!The
voidprefix correctly marks the fire-and-forgetloginWithTokencall in the reconnection handler. This is an event callback where the promise result cannot be propagated, and thevoidmakes the intent explicit for the linter.ee/packages/federation-matrix/tsconfig.json (1)
8-8: LGTM!Broadening from a single entry file to
"include": ["./src/**/*"]ensures all source files are type-checked, which is necessary for the TypeScript-aware ESLint rules to function across the package.ee/packages/federation-matrix/src/events/index.ts (1)
8-15: LGTM!
voidis correctly applied only to the twoasyncfunctions (ping,edus) while the synchronous event registration functions (message,reaction,member,room) are left as-is. This is consistent with the ESLintno-floating-promisesrule requirements.ee/packages/federation-matrix/tsconfig.build.json (1)
8-9: LGTM!The broader
includepattern aligns with the basetsconfig.jsonchange, and test files are properly excluded from the build output.ee/packages/presence/src/Presence.ts (1)
301-304:voidon broadcast is acceptable here.The presence status broadcast is non-critical and self-heals on the next status change. This is a reasonable fire-and-forget pattern.
packages/livechat/src/lib/hooks.ts (1)
138-138: Remainingvoid-wrapped calls are appropriate fire-and-forget patterns.
sendVisitorNavigation(Line 138): analytics-style page tracking, best-effort.i18next.changeLanguage(Line 290): language switch is non-critical.api[fn](...args)(Line 351): postMessage dispatch has no response channel, so fire-and-forget is the only option.Also applies to: 289-290, 351-351
ee/apps/ddp-streamer/src/DDPStreamer.ts (1)
69-69: Broadcast and connection-countvoidcalls are appropriate fire-and-forget.These are non-critical event notifications and periodic metric updates that self-heal. The
voidprefix correctly satisfies the lint rule here.Also applies to: 185-185, 191-191, 204-204, 215-215
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Proposed changes (including videos or screenshots)
Issue(s)
https://rocketchat.atlassian.net/browse/CORE-1803
Steps to test or reproduce
Further comments
Summary by CodeRabbit
New Features
Improvements
Note: the
no-misused-promisesis disabled in some ui packages as the components being used (and imported from other packages) do not accept thePromise<void>signature.Note 2 on lib downgrade: 2 packages had es2024. This lib is apparently newer than our ESLint parser (someday we'll update to eslint 9) so in order to apply the rule, i needed to downgrade a year. Hopefully nothing game-changing happened between 23/24