feat(apps): apps-engine split, flip the switch#40186
feat(apps): apps-engine split, flip the switch#40186d-gubert wants to merge 25 commits intofeat/apps-engine-split--pr2b-dynamic-import-mapfrom
Conversation
|
WalkthroughThis PR migrates the Rocket.Chat apps framework from the ChangesPackage Migration to
Sequence DiagramsequenceDiagram
participant AppCode as App Bridge/Service
participant OldPath as `@rocket.chat/apps-engine`
participant NewPath as `@rocket.chat/apps/dist`
participant AppOrch as IAppServerOrchestrator
AppCode->>OldPath: [Before] Import AppManager, Bridges, Storage
OldPath-->>AppCode: Legacy types & classes
AppCode->>NewPath: [After] Import from dist paths
NewPath-->>AppCode: Types & classes from new package
AppCode->>AppOrch: [Email Bridge] Inject orchestrator
AppOrch->>AppCode: Provide logging & settings access
AppCode->>AppCode: sendEmail with default From_Email
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
|
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 |
d-gubert
force-pushed
the
feat/apps-engine-split--pr2b-dynamic-import-map
branch
from
969ce6b to
af58d57
Compare
d-gubert
force-pushed
the
feat/apps-engine-split--pr2c-flip-the-switch
branch
from
1cc12df to
c9af208
Compare
d-gubert
force-pushed
the
feat/apps-engine-split--pr2b-dynamic-import-map
branch
from
af58d57 to
68964e4
Compare
d-gubert
force-pushed
the
feat/apps-engine-split--pr2c-flip-the-switch
branch
2 times, most recently
from
f45a000 to
b39d06f
Compare
|
@copilot check the errors and fix them https://github.com/RocketChat/Rocket.Chat/actions/runs/25027766915/job/73302665636?pr=40186 Make atomic commits, don't just commit every file at once. |
Fixed in commits The build was failing because the cleanup commit ( The fix is split into two atomic commits:
The package now only contains |
Fixed in commit
Fixed by importing it as a value in |
d-gubert
force-pushed
the
feat/apps-engine-split--pr2c-flip-the-switch
branch
2 times, most recently
from
2b96982 to
a41200f
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## feat/apps-engine-split--pr2b-dynamic-import-map #40186 +/- ##
===================================================================================
+ Coverage 69.98% 70.01% +0.03%
===================================================================================
Files 3301 3301
Lines 120462 120463 +1
Branches 21583 21603 +20
===================================================================================
+ Hits 84311 84348 +37
+ Misses 32843 32824 -19
+ Partials 3308 3291 -17
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
d-gubert
force-pushed
the
feat/apps-engine-split--pr2b-dynamic-import-map
branch
2 times, most recently
from
a5aacd5 to
1a62b06
Compare
d-gubert
force-pushed
the
feat/apps-engine-split--pr2c-flip-the-switch
branch
2 times, most recently
from
e3b0a1c to
158024b
Compare
GetMessagesSortableFields is a runtime const array, not just a type. The previous refactor accidentally imported and re-exported it via 'import type' / 'export type', making it unavailable as a value. This caused TS1362 errors in RoomRead.ts where it is used at runtime (Array.includes and Array.join calls). Co-authored-by: d-gubert <1810309+d-gubert@users.noreply.github.com>
Co-authored-by: Copilot <copilot@github.com>
d-gubert
force-pushed
the
feat/apps-engine-split--pr2c-flip-the-switch
branch
from
b8cb7a9 to
a7dcbfc
Compare
coderabbitai
Bot
added
the
type: feature
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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)
apps/meteor/app/apps/server/bridges/email.ts (1)
15-18:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate
From_Emailbefore coercing it into the payload.If
From_Emailis unset,String(settings.get('From_Email'))becomes"undefined"/"null", and the bridge will send an invalidfromaddress on the exact path meant to recover from a missing app-provided sender.Suggested fix
if (!from) { this.orch.debugLog(`The app ${appId} didn't provide a from address, using the default one.`); - from = String(settings.get('From_Email')); + const defaultFrom = settings.get('From_Email'); + if (typeof defaultFrom !== 'string' || defaultFrom.trim() === '') { + throw new Error('From_Email is not configured'); + } + from = defaultFrom; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/meteor/app/apps/server/bridges/email.ts` around lines 15 - 18, The code coerces settings.get('From_Email') with String(...) which can produce "undefined"/"null"; update the fallback in the block handling the local variable from (the same section that calls this.orch.debugLog and uses appId) to first read and validate the value from settings.get('From_Email') (ensure it's not undefined/null/empty and is a plausibly formatted email) before assigning to from; if the setting is invalid, log an explicit error via this.orch.debugLog (including the appId and the invalid setting) and either keep from undefined so the caller can handle it or choose a safe hardcoded default, rather than using String(...) which yields invalid literals.
🧹 Nitpick comments (2)
apps/meteor/app/apps/server/bridges/contact.ts (1)
18-26: ⚡ Quick winPrefer the exported
VerifyContactChannelParamstype over the inline duplicate.
ContactBridgeis now imported from the same module that exportsVerifyContactChannelParams. The inline anonymous type on Line 19–25 is structurally identical to it; any future change toVerifyContactChannelParams(new field, rename) would silently miss this implementation.♻️ Proposed refactor
-import { ContactBridge } from '@rocket.chat/apps/dist/server/bridges/ContactBridge'; +import { ContactBridge } from '@rocket.chat/apps/dist/server/bridges/ContactBridge'; +import type { VerifyContactChannelParams } from '@rocket.chat/apps/dist/server/bridges/ContactBridge'; ... - async verifyContact( - verifyContactChannelParams: { - contactId: string; - field: string; - value: string; - visitorId: string; - roomId: string; - }, - appId: string, - ): Promise<void> { + async verifyContact(verifyContactChannelParams: VerifyContactChannelParams, appId: string): Promise<void> {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/meteor/app/apps/server/bridges/contact.ts` around lines 18 - 26, The verifyContact method currently declares an inline parameter type that duplicates the exported VerifyContactChannelParams; replace the inline anonymous type with the exported type by changing the parameter signature of verifyContact to accept verifyContactChannelParams: VerifyContactChannelParams (importing VerifyContactChannelParams from the same module that provides ContactBridge if not already imported), so the function signature uses the single source-of-truth type and will automatically reflect future shape changes.packages/apps-engine/src/definition/oauth2/OAuth2Client.ts (1)
209-217: ⚡ Quick winReplace deprecated
String.prototype.substrwithslice.
substris a legacy method flagged as deprecated in MDN/TC39. While it still works, since this code is being relocated to the new definition home, swapping it now is a near-zero-effort cleanup that keeps the moved surface free of deprecated APIs.♻️ Proposed fix
- if (url.endsWith('/')) { - return url.substr(0, url.length - 1); - } - return url; + return url.endsWith('/') ? url.slice(0, -1) : url;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/apps-engine/src/definition/oauth2/OAuth2Client.ts` around lines 209 - 217, In getBaseURLWithoutTrailingSlash, replace the deprecated String.prototype.substr usage with String.prototype.slice: when trimming the trailing slash from the url (in the method getBaseURLWithoutTrailingSlash and variable url), call slice(0, -1) instead of substr(0, url.length - 1) so the code avoids the deprecated API while preserving behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/meteor/app/apps/server/bridges/uiInteraction.ts`:
- Line 2: The code is using deep-imports like
'@rocket.chat/apps/dist/server/bridges/UiInteractionBridge' (see
UiInteractionBridge import in apps/server/bridges/uiInteraction.ts) which bypass
the package's exports map; update packages/apps/package.json to add explicit
"exports" subpath entries that expose the compiled dist paths used across the
repo (at minimum map "./dist/server/bridges/UiInteractionBridge" and the broader
"./dist/server/bridges/*" plus analogous entries for "./dist/server/storage/*",
"./dist/server/managers/*" and "./dist/client/*"), ensuring the exported targets
include both the JS entry and typings where applicable so existing imports
continue to resolve without relying on implicit build output layout.
In
`@packages/apps-engine/src/definition/externalComponent/IExternalComponentRoomInfo.ts`:
- Line 15: The interface IExternalComponentRoomInfo currently requires members:
Array<IExternalComponentUserInfo>, forcing callers to eagerly populate full
IExternalComponentUserInfo (including avatarUrl) for every member; change
members to an optional property (members?: Array<IExternalComponentUserInfo>) or
document a clear size/usage constraint to avoid heavy eager loading for large
rooms, and update any related codepaths that construct or consume
IExternalComponentRoomInfo (and any factories that populate avatarUrl) to handle
the optional members case gracefully.
In `@packages/apps-engine/src/definition/rooms/index.ts`:
- Line 29: The rooms barrel currently only re-exports types from
IGetMessagesOptions, so the runtime constant GetMessagesSortableFields isn't
exported and consumers must deep-import it; update the barrel to also export the
value (e.g., add an export for GetMessagesSortableFields or change the export to
include value exports) so GetMessagesSortableFields is available from the rooms
index alongside the existing IGetMessagesOptions types.
---
Outside diff comments:
In `@apps/meteor/app/apps/server/bridges/email.ts`:
- Around line 15-18: The code coerces settings.get('From_Email') with
String(...) which can produce "undefined"/"null"; update the fallback in the
block handling the local variable from (the same section that calls
this.orch.debugLog and uses appId) to first read and validate the value from
settings.get('From_Email') (ensure it's not undefined/null/empty and is a
plausibly formatted email) before assigning to from; if the setting is invalid,
log an explicit error via this.orch.debugLog (including the appId and the
invalid setting) and either keep from undefined so the caller can handle it or
choose a safe hardcoded default, rather than using String(...) which yields
invalid literals.
---
Nitpick comments:
In `@apps/meteor/app/apps/server/bridges/contact.ts`:
- Around line 18-26: The verifyContact method currently declares an inline
parameter type that duplicates the exported VerifyContactChannelParams; replace
the inline anonymous type with the exported type by changing the parameter
signature of verifyContact to accept verifyContactChannelParams:
VerifyContactChannelParams (importing VerifyContactChannelParams from the same
module that provides ContactBridge if not already imported), so the function
signature uses the single source-of-truth type and will automatically reflect
future shape changes.
In `@packages/apps-engine/src/definition/oauth2/OAuth2Client.ts`:
- Around line 209-217: In getBaseURLWithoutTrailingSlash, replace the deprecated
String.prototype.substr usage with String.prototype.slice: when trimming the
trailing slash from the url (in the method getBaseURLWithoutTrailingSlash and
variable url), call slice(0, -1) instead of substr(0, url.length - 1) so the
code avoids the deprecated API while preserving behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 89260495-7c7f-4638-b4b8-8addefad3e9b
⛔ Files ignored due to path filters (2)
packages/apps/deno-runtime/deno.lockis excluded by!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (82)
.github/actions/build-docker/action.ymlapps/meteor/app/apps/server/bridges/activation.tsapps/meteor/app/apps/server/bridges/api.tsapps/meteor/app/apps/server/bridges/bridges.jsapps/meteor/app/apps/server/bridges/cloud.tsapps/meteor/app/apps/server/bridges/commands.tsapps/meteor/app/apps/server/bridges/contact.tsapps/meteor/app/apps/server/bridges/details.tsapps/meteor/app/apps/server/bridges/email.tsapps/meteor/app/apps/server/bridges/environmental.tsapps/meteor/app/apps/server/bridges/experimental.tsapps/meteor/app/apps/server/bridges/http.tsapps/meteor/app/apps/server/bridges/index.tsapps/meteor/app/apps/server/bridges/internal.tsapps/meteor/app/apps/server/bridges/internalFederation.tsapps/meteor/app/apps/server/bridges/livechat.tsapps/meteor/app/apps/server/bridges/messages.tsapps/meteor/app/apps/server/bridges/moderation.tsapps/meteor/app/apps/server/bridges/oauthApps.tsapps/meteor/app/apps/server/bridges/outboundCommunication.tsapps/meteor/app/apps/server/bridges/persistence.tsapps/meteor/app/apps/server/bridges/roles.tsapps/meteor/app/apps/server/bridges/rooms.tsapps/meteor/app/apps/server/bridges/scheduler.tsapps/meteor/app/apps/server/bridges/settings.tsapps/meteor/app/apps/server/bridges/thread.tsapps/meteor/app/apps/server/bridges/uiInteraction.tsapps/meteor/app/apps/server/bridges/uploads.tsapps/meteor/app/apps/server/bridges/users.tsapps/meteor/app/apps/server/bridges/videoConferences.tsapps/meteor/app/statistics/server/lib/getAppsStatistics.tsapps/meteor/client/apps/RealAppsEngineUIHost.tsapps/meteor/client/apps/orchestrator.tsapps/meteor/client/contexts/AppsContext.tsxapps/meteor/ee/app/license/server/canEnableApp.tsapps/meteor/ee/lib/misc/formatAppInstanceForRest.tsapps/meteor/ee/server/apps/communication/endpoints/appsCountHandler.tsapps/meteor/ee/server/apps/communication/rest.tsapps/meteor/ee/server/apps/cron.tsapps/meteor/ee/server/apps/orchestrator.jsapps/meteor/ee/server/apps/storage/AppFileSystemSourceStorage.tsapps/meteor/ee/server/apps/storage/AppGridFSSourceStorage.tsapps/meteor/ee/server/apps/storage/AppRealLogStorage.tsapps/meteor/ee/server/apps/storage/AppRealStorage.tsapps/meteor/ee/server/apps/storage/ConfigurableAppSourceStorage.tsapps/meteor/ee/tests/unit/server/apps/AppRealStorage.spec.tsapps/meteor/lib/apps/getInstallationSourceFromAppStorageItem.tsapps/meteor/server/services/apps-engine/service.tsapps/meteor/server/services/video-conference/service.tsapps/meteor/server/startup/migrations/v294.tsapps/meteor/server/startup/migrations/v307.tsapps/meteor/tests/end-to-end/apps/app-logs-export.tsapps/meteor/tests/end-to-end/apps/app-logs-general.tsapps/meteor/tests/end-to-end/apps/app-logs-nested-requests.tsapps/meteor/tests/end-to-end/apps/app-logs.tsapps/meteor/tests/mocks/client/marketplace.tsxapps/meteor/tests/mocks/data.tsapps/meteor/tests/unit/app/license/server/canEnableApp.spec.tsdocs/apps-engine-migration.mdee/apps/account-service/Dockerfileee/apps/authorization-service/Dockerfileee/apps/ddp-streamer/Dockerfileee/apps/omnichannel-transcript/Dockerfileee/apps/presence-service/Dockerfileee/apps/queue-worker/Dockerfilepackages/apps-engine/src/definition/accessors/IRoomRead.tspackages/apps-engine/src/definition/externalComponent/IExternalComponentRoomInfo.tspackages/apps-engine/src/definition/externalComponent/IExternalComponentState.tspackages/apps-engine/src/definition/externalComponent/IExternalComponentUserInfo.tspackages/apps-engine/src/definition/externalComponent/index.tspackages/apps-engine/src/definition/oauth2/OAuth2.tspackages/apps-engine/src/definition/oauth2/OAuth2Client.tspackages/apps-engine/src/definition/rooms/IGetMessagesOptions.tspackages/apps-engine/src/definition/rooms/index.tspackages/apps/package.jsonpackages/apps/src/AppsEngine.tspackages/apps/src/IAppServerOrchestrator.tspackages/apps/src/converters/IAppMessagesConverter.tspackages/apps/src/server/bridges/RoomBridge.tspackages/apps/src/server/oauth2/OAuth2Client.tspackages/core-services/package.jsonpackages/core-services/src/types/IAppsEngineService.ts
💤 Files with no reviewable changes (6)
- ee/apps/presence-service/Dockerfile
- ee/apps/omnichannel-transcript/Dockerfile
- ee/apps/authorization-service/Dockerfile
- ee/apps/account-service/Dockerfile
- ee/apps/ddp-streamer/Dockerfile
- ee/apps/queue-worker/Dockerfile
📜 Review details
🧰 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/client/contexts/AppsContext.tsxapps/meteor/app/apps/server/bridges/internalFederation.tspackages/apps-engine/src/definition/oauth2/OAuth2.tsapps/meteor/ee/server/apps/communication/endpoints/appsCountHandler.tsapps/meteor/tests/end-to-end/apps/app-logs-nested-requests.tsapps/meteor/app/apps/server/bridges/experimental.tsapps/meteor/app/apps/server/bridges/bridges.jsapps/meteor/app/apps/server/bridges/roles.tsapps/meteor/ee/server/apps/storage/AppFileSystemSourceStorage.tsapps/meteor/client/apps/orchestrator.tspackages/apps-engine/src/definition/externalComponent/index.tsapps/meteor/tests/end-to-end/apps/app-logs-export.tsapps/meteor/tests/mocks/data.tspackages/core-services/src/types/IAppsEngineService.tspackages/apps-engine/src/definition/rooms/index.tsapps/meteor/ee/tests/unit/server/apps/AppRealStorage.spec.tsapps/meteor/ee/server/apps/storage/AppGridFSSourceStorage.tsapps/meteor/tests/mocks/client/marketplace.tsxpackages/apps-engine/src/definition/externalComponent/IExternalComponentUserInfo.tsapps/meteor/tests/end-to-end/apps/app-logs.tspackages/apps/src/server/oauth2/OAuth2Client.tsapps/meteor/ee/server/apps/storage/AppRealStorage.tsapps/meteor/lib/apps/getInstallationSourceFromAppStorageItem.tsapps/meteor/app/apps/server/bridges/moderation.tspackages/apps-engine/src/definition/externalComponent/IExternalComponentState.tsapps/meteor/tests/end-to-end/apps/app-logs-general.tspackages/apps/src/IAppServerOrchestrator.tsapps/meteor/tests/unit/app/license/server/canEnableApp.spec.tsapps/meteor/ee/server/apps/communication/rest.tsapps/meteor/app/apps/server/bridges/thread.tsapps/meteor/app/apps/server/bridges/outboundCommunication.tsapps/meteor/app/apps/server/bridges/settings.tsapps/meteor/client/apps/RealAppsEngineUIHost.tsapps/meteor/ee/server/apps/storage/ConfigurableAppSourceStorage.tsapps/meteor/ee/server/apps/orchestrator.jspackages/apps-engine/src/definition/externalComponent/IExternalComponentRoomInfo.tspackages/apps-engine/src/definition/accessors/IRoomRead.tsapps/meteor/app/apps/server/bridges/email.tsapps/meteor/app/apps/server/bridges/persistence.tsapps/meteor/app/apps/server/bridges/internal.tsapps/meteor/ee/server/apps/storage/AppRealLogStorage.tspackages/apps/src/AppsEngine.tsapps/meteor/app/apps/server/bridges/environmental.tsapps/meteor/app/apps/server/bridges/api.tsapps/meteor/app/apps/server/bridges/oauthApps.tsapps/meteor/app/apps/server/bridges/index.tspackages/apps-engine/src/definition/rooms/IGetMessagesOptions.tsapps/meteor/app/apps/server/bridges/activation.tsapps/meteor/server/services/apps-engine/service.tsapps/meteor/app/apps/server/bridges/http.tsapps/meteor/server/services/video-conference/service.tsapps/meteor/app/apps/server/bridges/details.tsapps/meteor/app/statistics/server/lib/getAppsStatistics.tsapps/meteor/app/apps/server/bridges/videoConferences.tspackages/apps/src/converters/IAppMessagesConverter.tsapps/meteor/ee/app/license/server/canEnableApp.tspackages/apps-engine/src/definition/oauth2/OAuth2Client.tsapps/meteor/server/startup/migrations/v307.tsapps/meteor/app/apps/server/bridges/contact.tsapps/meteor/app/apps/server/bridges/scheduler.tsapps/meteor/app/apps/server/bridges/rooms.tsapps/meteor/app/apps/server/bridges/users.tsapps/meteor/app/apps/server/bridges/commands.tsapps/meteor/ee/lib/misc/formatAppInstanceForRest.tsapps/meteor/app/apps/server/bridges/uiInteraction.tsapps/meteor/app/apps/server/bridges/livechat.tsapps/meteor/server/startup/migrations/v294.tsapps/meteor/app/apps/server/bridges/cloud.tsapps/meteor/app/apps/server/bridges/uploads.tsapps/meteor/app/apps/server/bridges/messages.tspackages/apps/src/server/bridges/RoomBridge.tsapps/meteor/ee/server/apps/cron.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:
apps/meteor/ee/tests/unit/server/apps/AppRealStorage.spec.tsapps/meteor/tests/unit/app/license/server/canEnableApp.spec.ts
🧠 Learnings (10)
📚 Learning: 2026-03-27T14:52:56.865Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39892
File: apps/meteor/client/views/room/contextualBar/Threads/Thread.tsx:150-155
Timestamp: 2026-03-27T14:52:56.865Z
Learning: In Rocket.Chat, there are two different `ModalBackdrop` components with different prop APIs. During review, confirm the import source: (1) `rocket.chat/fuselage` `ModalBackdrop` uses `ModalBackdropProps` based on `BoxProps` (so it supports `onClick` and other Box/DOM props) and does not have an `onDismiss` prop; (2) `rocket.chat/ui-client` `ModalBackdrop` uses a narrower props interface like `{ children?: ReactNode; onDismiss?: () => void }` and handles Escape keypress and outside mouse-up, and it does not forward arbitrary DOM props such as `onClick`. Flag mismatched props (e.g., `onDismiss` passed to the fuselage component or `onClick` passed to the ui-client component) and ensure the usage matches the correct component being imported.
Applied to files:
apps/meteor/client/contexts/AppsContext.tsxapps/meteor/tests/mocks/client/marketplace.tsx
📚 Learning: 2026-05-06T12:21:44.083Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 40256
File: apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx:121-149
Timestamp: 2026-05-06T12:21:44.083Z
Learning: Field wrappers in rocket.chat/fuselage-forms (Field, FieldLabel, FieldRow, FieldError, FieldHint) auto-create htmlFor/id associations, aria-describedby, and role="alert" for errors. Do not manually set htmlFor, id, aria-describedby, or role attributes when using these wrappers. This automatic wiring does not apply to plain rocket.chat/fuselage components, which require explicit ID wiring per the accessibility docs. In code reviews, prefer using fuselage-forms wrappers for form fields and verify there is no unnecessary manual ID/aria wiring in files that use these wrappers. If a component uses plain fuselage components, ensure proper id wiring as per docs.
Applied to files:
apps/meteor/client/contexts/AppsContext.tsxapps/meteor/app/apps/server/bridges/internalFederation.tspackages/apps-engine/src/definition/oauth2/OAuth2.tsapps/meteor/ee/server/apps/communication/endpoints/appsCountHandler.tsapps/meteor/tests/end-to-end/apps/app-logs-nested-requests.tsapps/meteor/app/apps/server/bridges/experimental.tsapps/meteor/app/apps/server/bridges/roles.tsapps/meteor/ee/server/apps/storage/AppFileSystemSourceStorage.tsapps/meteor/client/apps/orchestrator.tspackages/apps-engine/src/definition/externalComponent/index.tsapps/meteor/tests/end-to-end/apps/app-logs-export.tsapps/meteor/tests/mocks/data.tspackages/core-services/src/types/IAppsEngineService.tspackages/apps-engine/src/definition/rooms/index.tsapps/meteor/ee/tests/unit/server/apps/AppRealStorage.spec.tsapps/meteor/ee/server/apps/storage/AppGridFSSourceStorage.tsapps/meteor/tests/mocks/client/marketplace.tsxpackages/apps-engine/src/definition/externalComponent/IExternalComponentUserInfo.tsapps/meteor/tests/end-to-end/apps/app-logs.tspackages/apps/src/server/oauth2/OAuth2Client.tsapps/meteor/ee/server/apps/storage/AppRealStorage.tsapps/meteor/lib/apps/getInstallationSourceFromAppStorageItem.tsapps/meteor/app/apps/server/bridges/moderation.tspackages/apps-engine/src/definition/externalComponent/IExternalComponentState.tsapps/meteor/tests/end-to-end/apps/app-logs-general.tspackages/apps/src/IAppServerOrchestrator.tsapps/meteor/tests/unit/app/license/server/canEnableApp.spec.tsapps/meteor/ee/server/apps/communication/rest.tsapps/meteor/app/apps/server/bridges/thread.tsapps/meteor/app/apps/server/bridges/outboundCommunication.tsapps/meteor/app/apps/server/bridges/settings.tsapps/meteor/client/apps/RealAppsEngineUIHost.tsapps/meteor/ee/server/apps/storage/ConfigurableAppSourceStorage.tspackages/apps-engine/src/definition/externalComponent/IExternalComponentRoomInfo.tspackages/apps-engine/src/definition/accessors/IRoomRead.tsapps/meteor/app/apps/server/bridges/email.tsapps/meteor/app/apps/server/bridges/persistence.tsapps/meteor/app/apps/server/bridges/internal.tsapps/meteor/ee/server/apps/storage/AppRealLogStorage.tspackages/apps/src/AppsEngine.tsapps/meteor/app/apps/server/bridges/environmental.tsapps/meteor/app/apps/server/bridges/api.tsapps/meteor/app/apps/server/bridges/oauthApps.tsapps/meteor/app/apps/server/bridges/index.tspackages/apps-engine/src/definition/rooms/IGetMessagesOptions.tsapps/meteor/app/apps/server/bridges/activation.tsapps/meteor/server/services/apps-engine/service.tsapps/meteor/app/apps/server/bridges/http.tsapps/meteor/server/services/video-conference/service.tsapps/meteor/app/apps/server/bridges/details.tsapps/meteor/app/statistics/server/lib/getAppsStatistics.tsapps/meteor/app/apps/server/bridges/videoConferences.tspackages/apps/src/converters/IAppMessagesConverter.tsapps/meteor/ee/app/license/server/canEnableApp.tspackages/apps-engine/src/definition/oauth2/OAuth2Client.tsapps/meteor/server/startup/migrations/v307.tsapps/meteor/app/apps/server/bridges/contact.tsapps/meteor/app/apps/server/bridges/scheduler.tsapps/meteor/app/apps/server/bridges/rooms.tsapps/meteor/app/apps/server/bridges/users.tsapps/meteor/app/apps/server/bridges/commands.tsapps/meteor/ee/lib/misc/formatAppInstanceForRest.tsapps/meteor/app/apps/server/bridges/uiInteraction.tsapps/meteor/app/apps/server/bridges/livechat.tsapps/meteor/server/startup/migrations/v294.tsapps/meteor/app/apps/server/bridges/cloud.tsapps/meteor/app/apps/server/bridges/uploads.tsapps/meteor/app/apps/server/bridges/messages.tspackages/apps/src/server/bridges/RoomBridge.tsapps/meteor/ee/server/apps/cron.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.
Applied to files:
apps/meteor/app/apps/server/bridges/internalFederation.tspackages/apps-engine/src/definition/oauth2/OAuth2.tsapps/meteor/ee/server/apps/communication/endpoints/appsCountHandler.tsapps/meteor/tests/end-to-end/apps/app-logs-nested-requests.tsapps/meteor/app/apps/server/bridges/experimental.tsapps/meteor/app/apps/server/bridges/roles.tsapps/meteor/ee/server/apps/storage/AppFileSystemSourceStorage.tsapps/meteor/client/apps/orchestrator.tspackages/apps-engine/src/definition/externalComponent/index.tsapps/meteor/tests/end-to-end/apps/app-logs-export.tsapps/meteor/tests/mocks/data.tspackages/core-services/src/types/IAppsEngineService.tspackages/apps-engine/src/definition/rooms/index.tsapps/meteor/ee/tests/unit/server/apps/AppRealStorage.spec.tsapps/meteor/ee/server/apps/storage/AppGridFSSourceStorage.tspackages/apps-engine/src/definition/externalComponent/IExternalComponentUserInfo.tsapps/meteor/tests/end-to-end/apps/app-logs.tspackages/apps/src/server/oauth2/OAuth2Client.tsapps/meteor/ee/server/apps/storage/AppRealStorage.tsapps/meteor/lib/apps/getInstallationSourceFromAppStorageItem.tsapps/meteor/app/apps/server/bridges/moderation.tspackages/apps-engine/src/definition/externalComponent/IExternalComponentState.tsapps/meteor/tests/end-to-end/apps/app-logs-general.tspackages/apps/src/IAppServerOrchestrator.tsapps/meteor/tests/unit/app/license/server/canEnableApp.spec.tsapps/meteor/ee/server/apps/communication/rest.tsapps/meteor/app/apps/server/bridges/thread.tsapps/meteor/app/apps/server/bridges/outboundCommunication.tsapps/meteor/app/apps/server/bridges/settings.tsapps/meteor/client/apps/RealAppsEngineUIHost.tsapps/meteor/ee/server/apps/storage/ConfigurableAppSourceStorage.tspackages/apps-engine/src/definition/externalComponent/IExternalComponentRoomInfo.tspackages/apps-engine/src/definition/accessors/IRoomRead.tsapps/meteor/app/apps/server/bridges/email.tsapps/meteor/app/apps/server/bridges/persistence.tsapps/meteor/app/apps/server/bridges/internal.tsapps/meteor/ee/server/apps/storage/AppRealLogStorage.tspackages/apps/src/AppsEngine.tsapps/meteor/app/apps/server/bridges/environmental.tsapps/meteor/app/apps/server/bridges/api.tsapps/meteor/app/apps/server/bridges/oauthApps.tsapps/meteor/app/apps/server/bridges/index.tspackages/apps-engine/src/definition/rooms/IGetMessagesOptions.tsapps/meteor/app/apps/server/bridges/activation.tsapps/meteor/server/services/apps-engine/service.tsapps/meteor/app/apps/server/bridges/http.tsapps/meteor/server/services/video-conference/service.tsapps/meteor/app/apps/server/bridges/details.tsapps/meteor/app/statistics/server/lib/getAppsStatistics.tsapps/meteor/app/apps/server/bridges/videoConferences.tspackages/apps/src/converters/IAppMessagesConverter.tsapps/meteor/ee/app/license/server/canEnableApp.tspackages/apps-engine/src/definition/oauth2/OAuth2Client.tsapps/meteor/server/startup/migrations/v307.tsapps/meteor/app/apps/server/bridges/contact.tsapps/meteor/app/apps/server/bridges/scheduler.tsapps/meteor/app/apps/server/bridges/rooms.tsapps/meteor/app/apps/server/bridges/users.tsapps/meteor/app/apps/server/bridges/commands.tsapps/meteor/ee/lib/misc/formatAppInstanceForRest.tsapps/meteor/app/apps/server/bridges/uiInteraction.tsapps/meteor/app/apps/server/bridges/livechat.tsapps/meteor/server/startup/migrations/v294.tsapps/meteor/app/apps/server/bridges/cloud.tsapps/meteor/app/apps/server/bridges/uploads.tsapps/meteor/app/apps/server/bridges/messages.tspackages/apps/src/server/bridges/RoomBridge.tsapps/meteor/ee/server/apps/cron.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Applied to files:
apps/meteor/app/apps/server/bridges/internalFederation.tspackages/apps-engine/src/definition/oauth2/OAuth2.tsapps/meteor/ee/server/apps/communication/endpoints/appsCountHandler.tsapps/meteor/tests/end-to-end/apps/app-logs-nested-requests.tsapps/meteor/app/apps/server/bridges/experimental.tsapps/meteor/app/apps/server/bridges/roles.tsapps/meteor/ee/server/apps/storage/AppFileSystemSourceStorage.tsapps/meteor/client/apps/orchestrator.tspackages/apps-engine/src/definition/externalComponent/index.tsapps/meteor/tests/end-to-end/apps/app-logs-export.tsapps/meteor/tests/mocks/data.tspackages/core-services/src/types/IAppsEngineService.tspackages/apps-engine/src/definition/rooms/index.tsapps/meteor/ee/tests/unit/server/apps/AppRealStorage.spec.tsapps/meteor/ee/server/apps/storage/AppGridFSSourceStorage.tspackages/apps-engine/src/definition/externalComponent/IExternalComponentUserInfo.tsapps/meteor/tests/end-to-end/apps/app-logs.tspackages/apps/src/server/oauth2/OAuth2Client.tsapps/meteor/ee/server/apps/storage/AppRealStorage.tsapps/meteor/lib/apps/getInstallationSourceFromAppStorageItem.tsapps/meteor/app/apps/server/bridges/moderation.tspackages/apps-engine/src/definition/externalComponent/IExternalComponentState.tsapps/meteor/tests/end-to-end/apps/app-logs-general.tspackages/apps/src/IAppServerOrchestrator.tsapps/meteor/tests/unit/app/license/server/canEnableApp.spec.tsapps/meteor/ee/server/apps/communication/rest.tsapps/meteor/app/apps/server/bridges/thread.tsapps/meteor/app/apps/server/bridges/outboundCommunication.tsapps/meteor/app/apps/server/bridges/settings.tsapps/meteor/client/apps/RealAppsEngineUIHost.tsapps/meteor/ee/server/apps/storage/ConfigurableAppSourceStorage.tspackages/apps-engine/src/definition/externalComponent/IExternalComponentRoomInfo.tspackages/apps-engine/src/definition/accessors/IRoomRead.tsapps/meteor/app/apps/server/bridges/email.tsapps/meteor/app/apps/server/bridges/persistence.tsapps/meteor/app/apps/server/bridges/internal.tsapps/meteor/ee/server/apps/storage/AppRealLogStorage.tspackages/apps/src/AppsEngine.tsapps/meteor/app/apps/server/bridges/environmental.tsapps/meteor/app/apps/server/bridges/api.tsapps/meteor/app/apps/server/bridges/oauthApps.tsapps/meteor/app/apps/server/bridges/index.tspackages/apps-engine/src/definition/rooms/IGetMessagesOptions.tsapps/meteor/app/apps/server/bridges/activation.tsapps/meteor/server/services/apps-engine/service.tsapps/meteor/app/apps/server/bridges/http.tsapps/meteor/server/services/video-conference/service.tsapps/meteor/app/apps/server/bridges/details.tsapps/meteor/app/statistics/server/lib/getAppsStatistics.tsapps/meteor/app/apps/server/bridges/videoConferences.tspackages/apps/src/converters/IAppMessagesConverter.tsapps/meteor/ee/app/license/server/canEnableApp.tspackages/apps-engine/src/definition/oauth2/OAuth2Client.tsapps/meteor/server/startup/migrations/v307.tsapps/meteor/app/apps/server/bridges/contact.tsapps/meteor/app/apps/server/bridges/scheduler.tsapps/meteor/app/apps/server/bridges/rooms.tsapps/meteor/app/apps/server/bridges/users.tsapps/meteor/app/apps/server/bridges/commands.tsapps/meteor/ee/lib/misc/formatAppInstanceForRest.tsapps/meteor/app/apps/server/bridges/uiInteraction.tsapps/meteor/app/apps/server/bridges/livechat.tsapps/meteor/server/startup/migrations/v294.tsapps/meteor/app/apps/server/bridges/cloud.tsapps/meteor/app/apps/server/bridges/uploads.tsapps/meteor/app/apps/server/bridges/messages.tspackages/apps/src/server/bridges/RoomBridge.tsapps/meteor/ee/server/apps/cron.ts
📚 Learning: 2026-02-04T12:08:56.950Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 38374
File: apps/meteor/tests/end-to-end/apps/app-logs-nested-requests.ts:26-37
Timestamp: 2026-02-04T12:08:56.950Z
Learning: In the E2E tests under apps/meteor/tests/end-to-end/apps, prefer using a fixed sleep (e.g., 1s) instead of implementing polling or retry logic when waiting for asynchronous operations to complete. Tests should fail deterministically if the expected result isn't available after the sleep.
Applied to files:
apps/meteor/tests/end-to-end/apps/app-logs-nested-requests.tsapps/meteor/tests/end-to-end/apps/app-logs-export.tsapps/meteor/tests/end-to-end/apps/app-logs.tsapps/meteor/tests/end-to-end/apps/app-logs-general.ts
📚 Learning: 2026-03-16T11:50:11.087Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 39657
File: apps/meteor/tests/end-to-end/apps/app-resolve-visitor.ts:18-21
Timestamp: 2026-03-16T11:50:11.087Z
Learning: In end-to-end tests under apps/meteor/tests/end-to-end/apps, there is an established pattern: call createAgent() and makeAgentAvailable() immediately after updateSetting('Livechat_enabled', true) with no intermediate delay. Do not flag this sequence as a race condition or require a sleep/delay. This pattern is used across 10+ tests in the codebase.
Applied to files:
apps/meteor/tests/end-to-end/apps/app-logs-nested-requests.tsapps/meteor/tests/end-to-end/apps/app-logs-export.tsapps/meteor/tests/end-to-end/apps/app-logs.tsapps/meteor/tests/end-to-end/apps/app-logs-general.ts
📚 Learning: 2026-02-10T16:32:42.586Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38528
File: apps/meteor/client/startup/roles.ts:14-14
Timestamp: 2026-02-10T16:32:42.586Z
Learning: In Rocket.Chat's Meteor client code, DDP streams use EJSON and Date fields arrive as Date objects; do not manually construct new Date() in stream handlers (for example, in sdk.stream()). Only REST API responses return plain JSON where dates are strings, so implement explicit conversion there if needed. Apply this guidance to all TypeScript files under apps/meteor/client to ensure consistent date handling in DDP streams and REST responses.
Applied to files:
apps/meteor/client/apps/orchestrator.tsapps/meteor/client/apps/RealAppsEngineUIHost.ts
📚 Learning: 2026-02-24T19:22:48.358Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/omnichannel/omnichannel-send-pdf-transcript.spec.ts:66-67
Timestamp: 2026-02-24T19:22:48.358Z
Learning: In Playwright end-to-end tests (e.g., under apps/meteor/tests/e2e/...), prefer locating elements by translated text (getByText) and ARIA roles (getByRole) over data-qa attributes. If translation values change, update the corresponding test locators accordingly. Never use data-qa locators. This guideline applies to all Playwright e2e test specs in the repository and helps keep tests robust to UI text changes and accessible semantics.
Applied to files:
apps/meteor/ee/tests/unit/server/apps/AppRealStorage.spec.tsapps/meteor/tests/unit/app/license/server/canEnableApp.spec.ts
📚 Learning: 2026-03-06T18:10:15.268Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/code/CodeBlock.spec.tsx:47-68
Timestamp: 2026-03-06T18:10:15.268Z
Learning: In tests (especially those using testing-library/dom/jsdom) for Rocket.Chat components, the HTML <code> element has an implicit ARIA role of 'code'. Therefore, screen.getByRole('code') or screen.findByRole('code') will locate <code> elements even without a role attribute. Do not flag findByRole('code') as invalid in reviews; prefer using the implicit role instead of adding role="code" unless necessary for accessibility.
Applied to files:
apps/meteor/ee/tests/unit/server/apps/AppRealStorage.spec.tsapps/meteor/tests/unit/app/license/server/canEnableApp.spec.ts
📚 Learning: 2026-04-14T23:26:16.284Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 40159
File: .github/actions/setup-node/action.yml:44-59
Timestamp: 2026-04-14T23:26:16.284Z
Learning: In this repo’s Rocket.Chat CI composite actions (e.g., .github/actions/setup-node/action.yml), treat direct writes of resolved tool versions to $GITHUB_ENV as an accepted/intentional pattern. If the action updates $GITHUB_ENV using something like `echo "VAR=value" >> $GITHUB_ENV` (without multiline heredoc form and without explicit empty-value guards), do not flag it as missing newline/empty validation or as “incorrect” heredoc usage. This is considered acceptable because the resolved versions come from trusted/stable sources (e.g., package.json engines and/or .tool-versions) used by Rocket.Chat CI.
Applied to files:
.github/actions/build-docker/action.yml
🪛 Biome (2.4.14)
apps/meteor/app/apps/server/bridges/bridges.js
[error] 1-1: Illegal use of an import declaration outside of a module
(parse)
apps/meteor/ee/server/apps/orchestrator.js
[error] 6-6: Illegal use of an import declaration outside of a module
(parse)
🔇 Additional comments (64)
.github/actions/build-docker/action.yml (1)
103-104: Good migration follow-through on esbuild pruning.Adding the
@rocket.chat/appsesbuild cleanup alongside the existingapps-enginecleanup is consistent with the split and helps keep image size trimmed.apps/meteor/ee/server/apps/communication/endpoints/appsCountHandler.ts (1)
1-1: Import migration looks correct.
import typefrom@rocket.chat/apps/dist/server/AppManageris consistent with the split and keeps this usage type-only.apps/meteor/ee/server/apps/communication/rest.ts (1)
1-2: Type import updates are good.The new
@rocket.chat/apps/dist/...type imports match the migration intent and keep runtime behavior unchanged.apps/meteor/client/apps/RealAppsEngineUIHost.ts (1)
1-3: Import migration looks correct and consistent.The updated runtime and type-only imports align with the package split/migration target and keep typings explicit.
packages/apps/package.json (2)
6-6:typespath update is correct and consistent with the build output.This aligns with
tsconfig.jsongenerating declarations indist/.
19-22: Script and workspace install config changes look good for the split migration.
testunit/typecheckadditions andhoistingLimits: "workspaces"fit the monorepo transition cleanly.Also applies to: 58-60
apps/meteor/client/contexts/AppsContext.tsx (1)
1-1: Import migration is consistent and type-safe.Line 1 correctly switches
AppClientManagerto@rocket.chat/apps/dist/...as a type-only import, matching its interface-only usage.apps/meteor/client/apps/orchestrator.ts (1)
1-2: Dist-path imports are correctly wired for runtime and types.Lines 1-2 use the right split: value import for
AppClientManagerand type import forAppsEngineUIHost, consistent with how each is used in the class.apps/meteor/app/apps/server/bridges/environmental.ts (1)
1-2: LGTM — mechanical import-path migration.Import replaced correctly; class implementation and base-class contract are unchanged.
docs/apps-engine-migration.md (1)
29-29: LGTM — documentation entry is accurate and consistent.apps/meteor/app/apps/server/bridges/roles.ts (1)
1-3: LGTM — import-path migration only.apps/meteor/ee/app/license/server/canEnableApp.ts (1)
1-1: LGTM — type import migrated to dist path; interface matches usages.apps/meteor/app/apps/server/bridges/internal.ts (1)
1-5: LGTM — import-path migration only; known type mismatch pre-dates this PR.apps/meteor/app/apps/server/bridges/moderation.ts (1)
1-5: LGTM — bridge migrated to dist; definition-layer type imports fromapps-engine/definitionare intentionally retained per the migration strategy.apps/meteor/app/apps/server/bridges/experimental.ts (1)
1-3: LGTM — import-path migration only; empty base class confirmed.apps/meteor/app/apps/server/bridges/uiInteraction.ts (1)
1-4: LGTM — alias pattern cleanly avoids local name collision; definition-layer imports intentionally retained.apps/meteor/ee/server/apps/cron.ts (1)
1-1: Type-only import migration looks correct.The
ProxiedAppimport switch to the@rocket.chat/apps/distpath is consistent with the split/migration and keeps runtime behavior unchanged.apps/meteor/app/apps/server/bridges/details.ts (1)
2-2: Bridge import update is consistent and safe.Using
AppDetailChangesBridgefrom the dist path aligns with the migration and preserves the current inheritance/override contract.apps/meteor/app/apps/server/bridges/oauthApps.ts (1)
4-4: Import migration is good here.
OAuthAppsBridgenow coming from@rocket.chat/apps/dist/...matches the broader Apps-Engine split and keeps this bridge wiring intact.packages/core-services/package.json (1)
32-32: Dependency switch matches the migration plan.Replacing
@rocket.chat/apps-enginewith@rocket.chat/appsindevDependenciesis aligned with the split rollout.apps/meteor/ee/lib/misc/formatAppInstanceForRest.ts (1)
1-3: Type import updates are consistent.The moved type imports (
ProxiedApp,AppLicenseValidationResult,IAppStorageItem) to@rocket.chat/apps/dist/server/*fit the PR migration pattern without behavior changes.apps/meteor/tests/unit/app/license/server/canEnableApp.spec.ts (1)
1-2: Test import migration looks correct.Switching these types to
@rocket.chat/apps/dist/server/*keeps the spec aligned with the new package boundaries.apps/meteor/app/apps/server/bridges/bridges.js (1)
1-1: AppBridges import migration is in good shape.The new
@rocket.chat/apps/dist/server/bridges/AppBridgessource aligns with the bridge-layer split and preserves class extension behavior.apps/meteor/app/apps/server/bridges/commands.ts (1)
2-2: CommandBridge path update is consistent.Importing
CommandBridgefrom@rocket.chat/apps/dist/server/bridges/CommandBridgefits the migration and keeps current bridge implementation intact.apps/meteor/app/apps/server/bridges/http.ts (1)
2-3: LGTM — import path migration is correct.
IHttpBridgeRequestInfoandHttpBridgemoved to dist paths;IHttpResponseintentionally stays inapps-engine/definition/accessorsas a definition-layer type.apps/meteor/app/apps/server/bridges/uploads.ts (1)
2-2: LGTM — dist path migration forUploadBridge.apps/meteor/ee/server/apps/storage/ConfigurableAppSourceStorage.ts (1)
1-2: LGTM — dist path migration forAppSourceStorageandIAppStorageItem.apps/meteor/app/apps/server/bridges/thread.ts (1)
2-2: LGTM — dist path migration forThreadBridge.apps/meteor/app/apps/server/bridges/settings.ts (1)
2-2: LGTM — dist path migration forServerSettingBridge.apps/meteor/app/apps/server/bridges/cloud.ts (1)
2-3: LGTM —CloudWorkspaceBridgemigrated to dist;IWorkspaceTokencorrectly stays inapps-enginedefinition layer.apps/meteor/app/apps/server/bridges/users.ts (1)
2-2: LGTM — dist path migration forUserBridge.apps/meteor/app/statistics/server/lib/getAppsStatistics.ts (1)
2-2: ⚡ Quick winNo action needed.
AppInstallationSourceis a regularenum(notconst enum) that will be properly emitted as a runtime value in the compiled dist output. The import at line 2 and usage at line 46 are both correct.apps/meteor/server/services/apps-engine/service.ts (1)
2-3: LGTM — import migration correct.Both type-only imports resolve to the expected interfaces (confirmed by context snippets) and the usages at Lines 162–174 remain consistent.
apps/meteor/server/services/video-conference/service.ts (1)
2-2: LGTM — type-only import migration correct.apps/meteor/ee/server/apps/storage/AppRealLogStorage.ts (1)
1-3: LGTM — imports correctly migrated.
AppLogStorageis a runtime import (base class) and correctly kept as a value import;ILoggerStorageEntryandIAppLogStorageFindOptionsare correctlyimport type.apps/meteor/app/apps/server/bridges/api.ts (1)
2-3: LGTM — import migration correct.
ApiBridgeproperly kept as a value import (runtime base class);AppApicorrectly asimport type. Definition-layer types (IApiRequest,RequestMethod, etc.) correctly remain on@rocket.chat/apps-engine/definition.apps/meteor/ee/server/apps/storage/AppGridFSSourceStorage.ts (1)
1-2: LGTM — import migration correct.apps/meteor/app/apps/server/bridges/messages.ts (1)
2-3: LGTM — import migration correct.apps/meteor/tests/end-to-end/apps/app-logs-export.ts (1)
1-1: LGTM — import migration correct.apps/meteor/tests/end-to-end/apps/app-logs-nested-requests.ts (1)
1-1: Import migration looks correct and consistent with the split.
No functional concerns in this segment.apps/meteor/tests/end-to-end/apps/app-logs.ts (1)
1-1: Good path update for ILoggerStorageEntry.
This is aligned with the apps-engine → apps/dist migration.apps/meteor/tests/end-to-end/apps/app-logs-general.ts (1)
1-1: LGTM on this import migration.
No issues found in the changed segment.apps/meteor/ee/server/apps/orchestrator.js (1)
6-6: AppManager import switch looks correct.
No regression risk identified in this changed line.apps/meteor/lib/apps/getInstallationSourceFromAppStorageItem.ts (1)
1-1: Type import migration is clean.
Behavioral surface remains unchanged as expected.apps/meteor/app/apps/server/bridges/internalFederation.ts (1)
1-1: Looks good — federation bridge type import updated correctly.
No further concerns in this segment.packages/core-services/src/types/IAppsEngineService.ts (1)
1-2: Type import updates are consistent with the migration.
No interface-level issues introduced here.apps/meteor/app/apps/server/bridges/outboundCommunication.ts (1)
2-2: ⚡ Quick winNo action needed - import path is correct.
The import on line 2 is valid. The source file is named
OutboundMessagesBridge.ts(plural) inpackages/apps/src/server/bridges/, and the dist output preserves this naming. All internal imports across the codebase consistently useOutboundMessagesBridge(plural), confirming this is the canonical path.apps/meteor/app/apps/server/bridges/scheduler.ts (2)
4-4: LGTM — dist-path import is consistent with the PR-wide migration pattern.
93-93: LGTM — removing the explicitas Promise<Array<string>>cast is correct;Promise.alloverPromise<string>[]infersPromise<string[]>, which is structurally assignable to the declared return typePromise<void | Array<string>>.apps/meteor/ee/server/apps/storage/AppFileSystemSourceStorage.ts (1)
4-5: LGTM — both dist-path imports correctly match the abstract class and interface definitions confirmed in the context snippets.packages/apps/src/AppsEngine.ts (1)
23-25: LGTM — re-exports redirected to local./serverpaths align with the PR's consolidation strategy while keeping the public API surface unchanged.packages/apps/src/IAppServerOrchestrator.ts (1)
7-8: LGTM — resolvingAppManagerandAppSourceStoragefrom local server paths is consistent with the rest of the PR and keeps the interface surface unchanged.apps/meteor/app/apps/server/bridges/videoConferences.ts (1)
2-2: LGTM — dist-path import forVideoConferenceBridgeis consistent with the PR migration pattern.apps/meteor/app/apps/server/bridges/persistence.ts (1)
2-2: LGTM —PersistenceBridgedist-path import is consistent with the PR-wide migration.packages/apps-engine/src/definition/externalComponent/IExternalComponentRoomInfo.ts (1)
4-4: ⚡ Quick winNo action required. The
slugifiedNamefield is declared as required (string, notstring | undefined) onIRoom, soPick<IRoom, 'id' | 'slugifiedName'>correctly preserves it as a required field. The concern about silent optionality does not apply.> Likely an incorrect or invalid review comment.packages/apps-engine/src/definition/externalComponent/IExternalComponentUserInfo.ts (1)
9-14: ⚡ Quick winRemove the suggestion to make
avatarUrloptional.All implementations of
IExternalComponentUserInfoalready guarantee thatavatarUrlis a string:getClientUserInfo()uses an explicit|| ''fallback, the test factory and mock implementations always provide a value, and the room members mapping receives URLs fromgetAvatarURL()which returns a string for valid usernames. The requiredavatarUrl: stringinterface is correct and doesn't need to be optional.apps/meteor/server/startup/migrations/v294.ts (1)
15-15: ⚡ Quick winThe optional chaining removal is correct.
Apps.getManager()has a non-nullable return type (AppManager) in theIAppServerOrchestratorinterface, and the orchestrator proxy will throw an error if the orchestrator is not initialized rather than returningnull/undefined. The code properly guards against uninitialized orchestrator with theif (!Apps.self)check at line 9, making the subsequent call toApps.getManager()at line 15 safe. This pattern is consistently used in the codebase (see v307.ts for an identical example).packages/apps/src/converters/IAppMessagesConverter.ts (1)
7-9: LGTM — overload ordering and narrowing look correct.The three overloads (undefined-only → IMessage → union) are ordered most-specific first so TS resolves the right return type, and the optional
cacheKey?: objectis appropriately typed for identity-based caching.apps/meteor/app/apps/server/bridges/livechat.ts (3)
1-2: LGTM — dist-path import split.Sourcing the type (
IAppServerOrchestrator,IAppsLivechatMessage) from@rocket.chat/appswhile pulling the runtimeLivechatBridgeclass from the dist subpath is consistent with the rest of the bridges in this PR.
396-406: LGTM — cacheKey reuse across the batch.Passing the
livechatMessagescursor as the second argument leverages the newcacheKey?: objectoverload so every conversion in this batch shares the same cache entry by identity. Cursor identity is stable across.map().toArray(), so this is sound.
126-131: ⚡ Quick winType narrowing is correct—no action needed.
The
source?.type === 'app'check properly discriminates toIOmnichannelSourceApp, which declares all four properties (sidebarIcon,defaultIcon,label,destination). TypeScript will correctly narrow the type and not widen toany/unknown. The properties are optional on the discriminated variant, so spreading them with their values (includingundefinedif unset) is the intended behavior.packages/apps-engine/src/definition/oauth2/OAuth2Client.ts (1)
1-11: LGTM on the relocation.Imports are correctly re-pointed to relative paths within the definition layer, and
IOAuth2Client/IOAuth2ClientOptions/IAuthDatacontinue to be sourced from the siblingIOAuth2module. The class still implementsIOAuth2Clientper the contract inpackages/apps-engine/src/definition/oauth2/IOAuth2.ts:84-136.packages/apps-engine/src/definition/oauth2/OAuth2.ts (1)
1-1: LGTM.Local import correctly resolves to the relocated
OAuth2Clientin the same directory; factory contract is unchanged.packages/apps/src/server/oauth2/OAuth2Client.ts (1)
1-1: ⚡ Quick winNo changes needed—the deep-import subpath resolves correctly.
The re-export imports from
@rocket.chat/apps-engine/definition/oauth2/OAuth2Client, which resolves through legacy Node module resolution:@rocket.chat/apps-enginehas noexportsfield, so it falls back to the filesystem; thefilesarray includesdefinition/**, which covers the compiled subpath; and the source file properly exports bothOAuth2ClientandGrantType. The build compilessrc/definition/*todefinition/*at the package root, making the import valid.
| @@ -1,7 +1,7 @@ | |||
| import type { IAppServerOrchestrator } from '@rocket.chat/apps'; | |||
| import { UiInteractionBridge as AppsEngineUiInteractionBridge } from '@rocket.chat/apps/dist/server/bridges/UiInteractionBridge'; | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify that the dist paths imported by bridge files exist under packages/apps
fd -e ts -e js . packages/apps/dist/server/bridges/ 2>/dev/null | head -30
# Also confirm the package.json exports/typesVersions cover these paths
cat packages/apps/package.json | grep -A 30 '"exports"'
cat packages/apps/package.json | grep -A 30 '"typesVersions"'Repository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
#!/bin/bash
# First, find where the packages/apps directory is
find . -maxdepth 3 -type d -name "apps" | grep -E "(packages|node_modules)" | head -20Repository: RocketChat/Rocket.Chat
Length of output: 82
🏁 Script executed:
#!/bin/bash
# Check if packages/apps exists and what's in it
ls -la packages/ 2>/dev/null | head -20Repository: RocketChat/Rocket.Chat
Length of output: 1213
🏁 Script executed:
#!/bin/bash
# Find the actual uiInteraction.ts file to see what it imports
find . -name "uiInteraction.ts" -type f | grep -v node_modules | head -5Repository: RocketChat/Rocket.Chat
Length of output: 121
🏁 Script executed:
#!/bin/bash
# Look for any dist/server/bridges directories in the repository
find . -path "*/dist/server/bridges" -type d 2>/dev/nullRepository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
#!/bin/bash
# Read the actual import statement in the file
cat -n ./apps/meteor/app/apps/server/bridges/uiInteraction.ts | head -20Repository: RocketChat/Rocket.Chat
Length of output: 1112
🏁 Script executed:
#!/bin/bash
# Explore packages/apps structure
ls -la packages/apps/Repository: RocketChat/Rocket.Chat
Length of output: 734
🏁 Script executed:
#!/bin/bash
# Check packages/apps/package.json for exports
cat packages/apps/package.jsonRepository: RocketChat/Rocket.Chat
Length of output: 1806
🏁 Script executed:
#!/bin/bash
# Explore packages/apps/src structure to see if server/bridges exists
find packages/apps/src -type f -name "*.ts" | grep -i bridge | head -20Repository: RocketChat/Rocket.Chat
Length of output: 1140
🏁 Script executed:
#!/bin/bash
# Get full directory structure of packages/apps/src
tree -L 3 packages/apps/src 2>/dev/null || find packages/apps/src -type d | head -30Repository: RocketChat/Rocket.Chat
Length of output: 6932
🏁 Script executed:
#!/bin/bash
# Find all imports from `@rocket.chat/apps/dist` in the codebase
rg "@rocket.chat/apps/dist" --type ts --type tsx -n | head -40Repository: RocketChat/Rocket.Chat
Length of output: 94
🏁 Script executed:
#!/bin/bash
# Check if there's a tsconfig that defines the compilation output path
cat packages/apps/tsconfig.jsonRepository: RocketChat/Rocket.Chat
Length of output: 349
🏁 Script executed:
#!/bin/bash
# Find all imports from `@rocket.chat/apps/dist` pattern
rg "@rocket.chat/apps/dist" --type ts -n | head -50Repository: RocketChat/Rocket.Chat
Length of output: 7572
🏁 Script executed:
#!/bin/bash
# Count how many files in the bridges directory follow this import pattern
grep -r "@rocket.chat/apps/dist/server/bridges" apps/meteor/app/apps/server/bridges/ | wc -lRepository: RocketChat/Rocket.Chat
Length of output: 69
Add formal exports map to packages/apps/package.json to document and stabilize deep-import paths.
The deep imports from @rocket.chat/apps/dist/server/bridges/ and similar paths are valid—the source files exist at packages/apps/src/server/bridges/ and TypeScript will compile them to the expected dist structure. However, this pattern is used extensively across 40+ files (bridges, storage, managers, client) and currently bypasses the package's exports configuration. While the paths will work with the current build setup, adding explicit exports entries would formalize these APIs and protect against accidental breaking changes to the build output structure.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/meteor/app/apps/server/bridges/uiInteraction.ts` at line 2, The code is
using deep-imports like
'@rocket.chat/apps/dist/server/bridges/UiInteractionBridge' (see
UiInteractionBridge import in apps/server/bridges/uiInteraction.ts) which bypass
the package's exports map; update packages/apps/package.json to add explicit
"exports" subpath entries that expose the compiled dist paths used across the
repo (at minimum map "./dist/server/bridges/UiInteractionBridge" and the broader
"./dist/server/bridges/*" plus analogous entries for "./dist/server/storage/*",
"./dist/server/managers/*" and "./dist/client/*"), ensuring the exported targets
include both the JS entry and typings where applicable so existing imports
continue to resolve without relying on implicit build output layout.
There was a problem hiding this comment.
Meteor's bundler does not respect the exports keyword on package.json files. For now we'll need to keep the deep imports
There was a problem hiding this comment.
@d-gubert, thanks for the clarification! That makes total sense — since Meteor's bundler ignores the exports field in package.json, the deep dist/ imports are the correct approach here and no changes are needed.
✏️ Learnings added
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 40186
File: apps/meteor/app/apps/server/bridges/uiInteraction.ts:2-2
Timestamp: 2026-05-06T20:47:53.078Z
Learning: In the RocketChat/Rocket.Chat repository, Meteor's bundler does not respect the `exports` keyword in `package.json` files. Deep imports (e.g., `rocket.chat/apps/dist/server/bridges/UiInteractionBridge`) must be used instead of relying on `exports` subpath mappings. Do not suggest adding `exports` map entries to packages consumed by Meteor (e.g., `packages/apps/package.json`) as a fix for deep imports.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 40256
File: apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx:121-149
Timestamp: 2026-05-06T12:21:44.083Z
Learning: Field wrappers in rocket.chat/fuselage-forms (Field, FieldLabel, FieldRow, FieldError, FieldHint) auto-create htmlFor/id associations, aria-describedby, and role="alert" for errors. Do not manually set htmlFor, id, aria-describedby, or role attributes when using these wrappers. This automatic wiring does not apply to plain rocket.chat/fuselage components, which require explicit ID wiring per the accessibility docs. In code reviews, prefer using fuselage-forms wrappers for form fields and verify there is no unnecessary manual ID/aria wiring in files that use these wrappers. If a component uses plain fuselage components, ensure proper id wiring as per docs.
| * the list that contains all the users belonging | ||
| * to this room. | ||
| */ | ||
| members: Array<IExternalComponentUserInfo>; |
There was a problem hiding this comment.
members is required and non-optional — potential performance concern for large rooms.
Requiring members: Array<IExternalComponentUserInfo> (which itself requires avatarUrl per IExternalComponentUserInfo) means callers must eagerly load and resolve full user info for every member. For rooms with many members this is expensive on every external component invocation.
Consider members?: Array<IExternalComponentUserInfo> or documenting an explicit size constraint, so the API contract is clear about the cost.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/apps-engine/src/definition/externalComponent/IExternalComponentRoomInfo.ts`
at line 15, The interface IExternalComponentRoomInfo currently requires members:
Array<IExternalComponentUserInfo>, forcing callers to eagerly populate full
IExternalComponentUserInfo (including avatarUrl) for every member; change
members to an optional property (members?: Array<IExternalComponentUserInfo>) or
document a clear size/usage constraint to avoid heavy eager loading for large
rooms, and update any related codepaths that construct or consume
IExternalComponentRoomInfo (and any factories that populate avatarUrl) to handle
the optional members case gracefully.
There was a problem hiding this comment.
ExternalComponents as a feature is deprecated and unreachable. No worries here.
There was a problem hiding this comment.
@d-gubert, thanks for the context! Since ExternalComponents is deprecated and unreachable, the performance concern is moot. I'll note this for future reviews.
✏️ Learnings added
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 40186
File: packages/apps-engine/src/definition/externalComponent/IExternalComponentRoomInfo.ts:15-15
Timestamp: 2026-05-06T20:49:12.929Z
Learning: In the RocketChat/Rocket.Chat repository, the ExternalComponents feature (related to IExternalComponentRoomInfo, IExternalComponentUserInfo, and IExternalComponentState in packages/apps-engine/src/definition/externalComponent/) is deprecated and unreachable. Do not flag performance or API design concerns for code in this feature area.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 40256
File: apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx:121-149
Timestamp: 2026-05-06T12:21:44.083Z
Learning: Field wrappers in rocket.chat/fuselage-forms (Field, FieldLabel, FieldRow, FieldError, FieldHint) auto-create htmlFor/id associations, aria-describedby, and role="alert" for errors. Do not manually set htmlFor, id, aria-describedby, or role attributes when using these wrappers. This automatic wiring does not apply to plain rocket.chat/fuselage components, which require explicit ID wiring per the accessibility docs. In code reviews, prefer using fuselage-forms wrappers for form fields and verify there is no unnecessary manual ID/aria wiring in files that use these wrappers. If a component uses plain fuselage components, ensure proper id wiring as per docs.
| export type * from './IPreRoomUserLeave'; | ||
| export type * from './IPostRoomUserLeave'; | ||
| export type * from './IRoomUserLeaveContext'; | ||
| export type * from './IGetMessagesOptions'; |
There was a problem hiding this comment.
Re-export the new runtime sort-field constant from the rooms barrel.
Line 29 only re-exports the types from IGetMessagesOptions. GetMessagesSortableFields is a value export, so it is still missing from @rocket.chat/apps-engine/definition/rooms, which is why packages/apps/src/server/bridges/RoomBridge.ts now has to deep-import the file directly. That leaves the promoted room-query API incomplete at the barrel boundary.
Suggested fix
-export type * from './IGetMessagesOptions';
+export * from './IGetMessagesOptions';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/apps-engine/src/definition/rooms/index.ts` at line 29, The rooms
barrel currently only re-exports types from IGetMessagesOptions, so the runtime
constant GetMessagesSortableFields isn't exported and consumers must deep-import
it; update the barrel to also export the value (e.g., add an export for
GetMessagesSortableFields or change the export to include value exports) so
GetMessagesSortableFields is available from the rooms index alongside the
existing IGetMessagesOptions types.
|
Top of the stack was merged, including all the changes. Closing this one. |
Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Further comments
Related to the "Apps-Engine split" stack:
@rocket.chat/apps-engineto@rocket.chat/appsinternal package #40395Deprecated
Summary by CodeRabbit
New Features
Refactoring