chore: update Meteor and package versions to latest stable releases 3.4#38614
Merged
chore: update Meteor and package versions to latest stable releases 3.4#38614
Conversation
Contributor
|
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 |
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
WalkthroughUpdated Meteor runtime to METEOR@3.4 and bumped multiple package pins. Replaced runtime use of Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant Babel as "Babel\n(`@babel/core` transformSync)"
participant DB
Client->>Server: Create or update integration with script
Server->>Babel: transformSync(script, { presets: [preset-env], compact, minified, comments:false })
Babel-->>Server: result.code or throw error
alt compile success
Server->>DB: store compiled script and integration record
Server-->>Client: success response
else compile error
Server->>DB: store error state on integration
Server-->>Client: error response
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
🚥 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #38614 +/- ##
===========================================
+ Coverage 70.53% 70.57% +0.04%
===========================================
Files 3271 3273 +2
Lines 116804 116880 +76
Branches 21064 21202 +138
===========================================
+ Hits 82382 82485 +103
+ Misses 32369 32339 -30
- Partials 2053 2056 +3
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Contributor
📦 Docker Image Size Report📈 Changes
📊 Historical Trend---
config:
theme: "dark"
xyChart:
width: 900
height: 400
---
xychart
title "Image Size Evolution by Service (Last 30 Days + This PR)"
x-axis ["11/27 22:32", "11/28 19:05", "12/01 23:01", "12/02 21:57", "12/03 21:00", "12/04 18:17", "12/05 21:56", "12/08 20:15", "12/09 22:17", "12/10 23:26", "12/11 21:56", "12/12 22:45", "12/13 01:34", "12/15 22:31", "12/16 22:18", "12/17 21:04", "12/18 23:12", "12/19 23:27", "12/20 21:03", "12/22 18:54", "12/23 16:16", "12/24 19:38", "12/25 17:51", "12/26 13:18", "12/29 19:01", "12/30 20:52", "02/12 22:57", "02/13 22:38", "02/16 14:04", "02/18 20:20", "02/18 20:24 (PR)"]
y-axis "Size (GB)" 0 --> 0.5
line "account-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
line "authorization-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
line "ddp-streamer-service" [0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12]
line "omnichannel-transcript-service" [0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13]
line "presence-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
line "queue-worker-service" [0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13]
line "rocketchat" [0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.35]
Statistics (last 30 days):
ℹ️ About this reportThis report compares Docker image sizes from this build against the
Updated: Wed, 18 Feb 2026 20:24:20 GMT |
…cript compilation babel-compiler is now devOnly in Meteor 3.4, causing runtime crash: TypeError: Cannot read properties of undefined (reading 'Babel') Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
babel-compiler is devOnly in Meteor 3.4, so listing it in .meteor/packages causes a runtime reference to Package['babel-compiler'] in the bundle while the package is stripped from production builds. ecmascript already pulls it in as a build dependency. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
These packages are needed at runtime to compile integration scripts, so they must be in dependencies (not devDependencies) for Meteor to include them in the production bundle. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Meteor's module system doesn't resolve string-based preset names at runtime. Import the preset module directly so it gets bundled. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Member
Author
|
/jira ARCH-2083 |
babel-compiler is no longer in the production bundle (devOnly in Meteor 3.4), so the swc-core cleanup path no longer exists. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/meteor/app/integrations/server/methods/incoming/addIncomingIntegration.ts (1)
127-127: Consider using@rocket.chat/toolspickinstead ofunderscore.The other integration files (
updateIncomingIntegration.ts,validateOutgoingIntegration.ts) usepickfrom@rocket.chat/tools. This file still uses_.pickfromunderscore. For consistency, consider aligning with the other files.♻️ Proposed refactor
-import _ from 'underscore'; +import { pick } from '@rocket.chat/tools';-integrationData.scriptError = e instanceof Error ? _.pick(e, 'name', 'message', 'stack') : undefined; +integrationData.scriptError = e instanceof Error ? pick(e, 'name', 'message', 'stack') : undefined;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/app/integrations/server/methods/incoming/addIncomingIntegration.ts` at line 127, Replace the use of underscore's _.pick with the shared utility pick from `@rocket.chat/tools` for consistency: add an import for pick from '@rocket.chat/tools' at the top of addIncomingIntegration.ts and change the assignment integrationData.scriptError = e instanceof Error ? _.pick(e, 'name', 'message', 'stack') : undefined to use pick(e, 'name', 'message', 'stack') instead; ensure no other references to underscore remain in this function/file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/meteor/app/integrations/server/lib/validateOutgoingIntegration.ts`:
- Around line 184-191: The preset-env usage in the transformSync call that
produces integrationData.scriptCompiled should explicitly disable module
transformation to mirror the fix in updateIncomingIntegration.ts; update the
transformSync invocation that uses presetEnv (or the presetEnv definition) so it
includes { modules: false } when passing presets to transformSync for
integration.script, ensuring compiled output doesn't rewrite ES modules.
In
`@apps/meteor/app/integrations/server/methods/incoming/addIncomingIntegration.ts`:
- Around line 116-123: The preset-env configuration used when calling
transformSync(integration.script, { presets: [presetEnv], ... }) can transform
ES modules and break the isolated-vm sandbox; update the transform invocation or
the presetEnv config to disable module transformation by setting modules: false
(e.g., ensure presetEnv includes { modules: false }) so that transformSync keeps
module syntax intact and then assign the compiled code to
integrationData.scriptCompiled as before; update references in the transformSync
call and any presetEnv variable definition so the change applies where
transformSync is invoked.
---
Nitpick comments:
In
`@apps/meteor/app/integrations/server/methods/incoming/addIncomingIntegration.ts`:
- Line 127: Replace the use of underscore's _.pick with the shared utility pick
from `@rocket.chat/tools` for consistency: add an import for pick from
'@rocket.chat/tools' at the top of addIncomingIntegration.ts and change the
assignment integrationData.scriptError = e instanceof Error ? _.pick(e, 'name',
'message', 'stack') : undefined to use pick(e, 'name', 'message', 'stack')
instead; ensure no other references to underscore remain in this function/file.
🪄 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: 9c6f7fb5-2a19-4dc7-a375-aa18c8dd527a
📒 Files selected for processing (7)
apps/meteor/.meteor/packagesapps/meteor/.meteor/releaseapps/meteor/.meteor/versionsapps/meteor/app/integrations/server/lib/validateOutgoingIntegration.tsapps/meteor/app/integrations/server/methods/incoming/addIncomingIntegration.tsapps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.tsapps/meteor/package.json
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: 🚢 Build Docker (arm64, account-service, presence-service, omnichannel-transcript-service, cove...
- GitHub Check: 🚢 Build Docker (amd64, account-service, presence-service, omnichannel-transcript-service, cove...
- GitHub Check: 🚢 Build Docker (arm64, authorization-service, queue-worker-service, ddp-streamer-service, cove...
- GitHub Check: 🚢 Build Docker (amd64, authorization-service, queue-worker-service, ddp-streamer-service, cove...
- GitHub Check: cubic · AI code reviewer
- GitHub Check: 🔎 Code Check / TypeScript
- GitHub Check: 🔨 Test Unit / Unit Tests
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🧰 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:
apps/meteor/app/integrations/server/lib/validateOutgoingIntegration.tsapps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.tsapps/meteor/app/integrations/server/methods/incoming/addIncomingIntegration.ts
🧠 Learnings (8)
📓 Common learnings
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 0
File: :0-0
Timestamp: 2026-02-24T19:05:56.710Z
Learning: In Rocket.Chat PRs, keep feature PRs free of unrelated lockfile-only dependency bumps; prefer reverting lockfile drift or isolating such bumps into a separate "chore" commit/PR, and always use yarn install --immutable with the Yarn version pinned in package.json via Corepack.
Learnt from: smirk-dev
Repo: RocketChat/Rocket.Chat PR: 39625
File: apps/meteor/app/api/server/v1/push.ts:85-97
Timestamp: 2026-03-14T14:58:58.834Z
Learning: In RocketChat/Rocket.Chat, the `push.token` POST/DELETE endpoints in `apps/meteor/app/api/server/v1/push.ts` were already migrated to the chained router API pattern on `develop` prior to PR `#39625`. `cleanTokenResult` (which strips `authToken` and returns `PushTokenResult`) and `isPushTokenPOSTProps`/`isPushTokenDELETEProps` validators already exist on `develop`. PR `#39625` only migrates `push.get` and `push.info` to the chained pattern. Do not flag `cleanTokenResult` or `PushTokenResult` as newly introduced behavior-breaking changes when reviewing this PR.
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 0
File: :0-0
Timestamp: 2026-02-24T19:05:56.710Z
Learning: Rocket.Chat repo context: When a workspace manifest on develop already pins a dependency version (e.g., packages/web-ui-registration → "rocket.chat/ui-contexts": "27.0.1"), a lockfile change in a feature PR that upgrades only that dependency’s resolution is considered a manifest-driven sync and can be kept, preferably as a small "chore: sync yarn.lock with manifests" commit.
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38974
File: apps/meteor/app/api/server/v1/im.ts:220-221
Timestamp: 2026-02-24T19:09:09.561Z
Learning: In RocketChat/Rocket.Chat OpenAPI migration PRs for apps/meteor/app/api/server/v1 endpoints, maintainers prefer to avoid any logic changes; style-only cleanups (like removing inline comments) may be deferred to follow-ups to keep scope tight.
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38913
File: packages/ddp-client/src/legacy/types/SDKLegacy.ts:34-34
Timestamp: 2026-02-25T20:10:16.987Z
Learning: In the RocketChat/Rocket.Chat monorepo, packages/ddp-client and apps/meteor do not use TypeScript project references. Module augmentations in apps/meteor (e.g., declare module 'rocket.chat/rest-typings') are not visible when compiling packages/ddp-client in isolation, which is why legacy SDK methods that depend on OperationResult types for OpenAPI-migrated endpoints must remain commented out.
📚 Learning: 2026-02-25T20:10:16.987Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38913
File: packages/ddp-client/src/legacy/types/SDKLegacy.ts:34-34
Timestamp: 2026-02-25T20:10:16.987Z
Learning: In the RocketChat/Rocket.Chat monorepo, packages/ddp-client and apps/meteor do not use TypeScript project references. Module augmentations in apps/meteor (e.g., declare module 'rocket.chat/rest-typings') are not visible when compiling packages/ddp-client in isolation, which is why legacy SDK methods that depend on OperationResult types for OpenAPI-migrated endpoints must remain commented out.
Applied to files:
apps/meteor/.meteor/releaseapps/meteor/package.jsonapps/meteor/app/integrations/server/lib/validateOutgoingIntegration.tsapps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.tsapps/meteor/app/integrations/server/methods/incoming/addIncomingIntegration.tsapps/meteor/.meteor/versionsapps/meteor/.meteor/packages
📚 Learning: 2026-02-24T19:05:56.710Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 0
File: :0-0
Timestamp: 2026-02-24T19:05:56.710Z
Learning: Rocket.Chat repo context: When a workspace manifest on develop already pins a dependency version (e.g., packages/web-ui-registration → "rocket.chat/ui-contexts": "27.0.1"), a lockfile change in a feature PR that upgrades only that dependency’s resolution is considered a manifest-driven sync and can be kept, preferably as a small "chore: sync yarn.lock with manifests" commit.
Applied to files:
apps/meteor/package.jsonapps/meteor/.meteor/versions
📚 Learning: 2026-03-20T13:51:23.302Z
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 39553
File: apps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.ts:179-181
Timestamp: 2026-03-20T13:51:23.302Z
Learning: In `apps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.ts`, the truthiness guards `...(integration.avatar && { avatar })`, `...(integration.emoji && { emoji })`, `...(integration.alias && { alias })`, and `...(integration.script && { script })` in the `$set` payload of `updateIncomingIntegration` are intentional. Empty-string values for these fields should NOT overwrite the stored value — only truthy values are persisted. Do not flag these as bugs preventing explicit clears.
Applied to files:
apps/meteor/app/integrations/server/lib/validateOutgoingIntegration.tsapps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.tsapps/meteor/app/integrations/server/methods/incoming/addIncomingIntegration.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/integrations/server/lib/validateOutgoingIntegration.tsapps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.tsapps/meteor/app/integrations/server/methods/incoming/addIncomingIntegration.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/integrations/server/lib/validateOutgoingIntegration.tsapps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.tsapps/meteor/app/integrations/server/methods/incoming/addIncomingIntegration.ts
📚 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:
apps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.ts
📚 Learning: 2026-02-24T19:09:09.561Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38974
File: apps/meteor/app/api/server/v1/im.ts:220-221
Timestamp: 2026-02-24T19:09:09.561Z
Learning: In RocketChat/Rocket.Chat OpenAPI migration PRs for apps/meteor/app/api/server/v1 endpoints, maintainers prefer to avoid any logic changes; style-only cleanups (like removing inline comments) may be deferred to follow-ups to keep scope tight.
Applied to files:
apps/meteor/.meteor/versionsapps/meteor/.meteor/packages
🔇 Additional comments (8)
apps/meteor/.meteor/release (1)
1-1: LGTM!Meteor runtime upgrade to 3.4 aligns with the PR objectives and the accompanying dependency updates.
apps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.ts (2)
1-3: LGTM on imports.The
@ts-expect-errorcomment is appropriate since@babel/preset-envlacks type declarations. Importing the preset directly (rather than by string name) ensures Meteor bundles it correctly.
94-101: No changes needed. The use of@babel/preset-envwithout explicitmodules: falseis correct here.User integration scripts contain no import/export statements—just plain class definitions. Babel's preset-env with its default
modules: 'auto'setting does not transform code without imports/exports, so theScriptclass remains globally accessible as expected by the sandbox. The identical pattern already works reliably inaddIncomingIntegration.tsand is tested in production.apps/meteor/app/integrations/server/lib/validateOutgoingIntegration.ts (1)
1-3: Imports are consistent with the other integration files.apps/meteor/.meteor/versions (1)
1-90: LGTM!Version pins are consistent with the Meteor 3.4 upgrade. The presence of
babel-compiler@7.13.0in versions is expected—it's resolved as a transitive dependency but won't be imported at runtime since it was removed from.meteor/packages.apps/meteor/.meteor/packages (1)
10-56: LGTM!Package versions are correctly bumped for Meteor 3.4. The removal of
babel-compilerfrom this file (not shown in diff but confirmed in PR objectives) is the key change that prevents the runtimeTypeErrorwhen readingBabel.apps/meteor/app/integrations/server/methods/incoming/addIncomingIntegration.ts (1)
1-3: Imports are consistent with the other integration files.apps/meteor/package.json (1)
72-73: @babel/core and@babel/preset-envversions 7.28.6 are published on npm and valid.Both versions exist and are available for installation.
Comment on lines
+184
to
+191
| const result = transformSync(integration.script, { | ||
| presets: [presetEnv], | ||
| compact: true, | ||
| minified: true, | ||
| comments: false, | ||
| }); | ||
|
|
||
| integrationData.scriptCompiled = Babel.compile(integration.script, babelOptions).code; | ||
| integrationData.scriptCompiled = result?.code ?? undefined; |
Contributor
There was a problem hiding this comment.
Same modules: false consideration applies here.
This file has the same preset-env configuration as updateIncomingIntegration.ts. If the module transformation issue is confirmed, apply the same fix here:
🛠️ Proposed fix
const result = transformSync(integration.script, {
- presets: [presetEnv],
+ presets: [[presetEnv, { modules: false }]],
compact: true,
minified: true,
comments: false,
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const result = transformSync(integration.script, { | |
| presets: [presetEnv], | |
| compact: true, | |
| minified: true, | |
| comments: false, | |
| }); | |
| integrationData.scriptCompiled = Babel.compile(integration.script, babelOptions).code; | |
| integrationData.scriptCompiled = result?.code ?? undefined; | |
| const result = transformSync(integration.script, { | |
| presets: [[presetEnv, { modules: false }]], | |
| compact: true, | |
| minified: true, | |
| comments: false, | |
| }); | |
| integrationData.scriptCompiled = result?.code ?? undefined; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/meteor/app/integrations/server/lib/validateOutgoingIntegration.ts`
around lines 184 - 191, The preset-env usage in the transformSync call that
produces integrationData.scriptCompiled should explicitly disable module
transformation to mirror the fix in updateIncomingIntegration.ts; update the
transformSync invocation that uses presetEnv (or the presetEnv definition) so it
includes { modules: false } when passing presets to transformSync for
integration.script, ensuring compiled output doesn't rewrite ES modules.
Comment on lines
+116
to
+123
| const result = transformSync(integration.script, { | ||
| presets: [presetEnv], | ||
| compact: true, | ||
| minified: true, | ||
| comments: false, | ||
| }); | ||
|
|
||
| integrationData.scriptCompiled = Babel.compile(integration.script, babelOptions).code; | ||
| integrationData.scriptCompiled = result?.code ?? undefined; |
Contributor
There was a problem hiding this comment.
Same modules: false consideration applies here.
This file shares the same preset-env configuration. Apply the same fix if the module transformation is confirmed to break the isolated-vm sandbox:
🛠️ Proposed fix
const result = transformSync(integration.script, {
- presets: [presetEnv],
+ presets: [[presetEnv, { modules: false }]],
compact: true,
minified: true,
comments: false,
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const result = transformSync(integration.script, { | |
| presets: [presetEnv], | |
| compact: true, | |
| minified: true, | |
| comments: false, | |
| }); | |
| integrationData.scriptCompiled = Babel.compile(integration.script, babelOptions).code; | |
| integrationData.scriptCompiled = result?.code ?? undefined; | |
| const result = transformSync(integration.script, { | |
| presets: [[presetEnv, { modules: false }]], | |
| compact: true, | |
| minified: true, | |
| comments: false, | |
| }); | |
| integrationData.scriptCompiled = result?.code ?? undefined; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/meteor/app/integrations/server/methods/incoming/addIncomingIntegration.ts`
around lines 116 - 123, The preset-env configuration used when calling
transformSync(integration.script, { presets: [presetEnv], ... }) can transform
ES modules and break the isolated-vm sandbox; update the transform invocation or
the presetEnv config to disable module transformation by setting modules: false
(e.g., ensure presetEnv includes { modules: false }) so that transformSync keeps
module syntax intact and then assign the compiled code to
integrationData.scriptCompiled as before; update references in the transformSync
call and any presetEnv variable definition so the change applies where
transformSync is invoked.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 6 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/actions/build-docker/action.yml">
<violation number="1" location=".github/actions/build-docker/action.yml:85">
P2: Guard these `find` cleanup commands against missing directories; otherwise optional package layout differences can break Docker builds.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Comment on lines
85
to
87
| find /tmp/build/bundle/programs/server/npm/node_modules/@img -type d -name 'sharp-*' -not -name "*-linuxmusl-${swc_arch}" -exec rm -rf {} + | ||
|
|
||
| find /tmp/build/bundle/programs/server/npm/node_modules/@napi-rs -type d -name 'pinyin-linux-*' -not -name "*-linux-${swc_arch}-*" -exec rm -rf {} + |
Contributor
There was a problem hiding this comment.
P2: Guard these find cleanup commands against missing directories; otherwise optional package layout differences can break Docker builds.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/actions/build-docker/action.yml, line 85:
<comment>Guard these `find` cleanup commands against missing directories; otherwise optional package layout differences can break Docker builds.</comment>
<file context>
@@ -82,13 +82,13 @@ runs:
fi
- # find /tmp/build/bundle/programs/server/npm/node_modules/@img -type d -name 'sharp-*' -not -name "*-linuxmusl-${swc_arch}" -exec rm -rf {} +
+ find /tmp/build/bundle/programs/server/npm/node_modules/@img -type d -name 'sharp-*' -not -name "*-linuxmusl-${swc_arch}" -exec rm -rf {} +
- # find /tmp/build/bundle/programs/server/npm/node_modules/@napi-rs -type d -name 'pinyin-linux-*' -not -name "*-linux-${swc_arch}-*" -exec rm -rf {} +
</file context>
ggazzo
commented
Apr 8, 2026
KevLehman
reviewed
Apr 8, 2026
ggazzo
commented
Apr 8, 2026
ggazzo
commented
Apr 8, 2026
Co-authored-by: Guilherme Gazzo <guilhermegazzo@gmail.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/meteor/app/integrations/server/lib/validateOutgoingIntegration.ts (1)
191-191: Remove the inline TODO before merge.Please track this in ARCH-2092 or a follow-up issue instead of leaving it in the implementation. As per coding guidelines, "Avoid code comments in the implementation".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/app/integrations/server/lib/validateOutgoingIntegration.ts` at line 191, Remove the inline TODO comment ("// TODO: Webhook Integration Editor should inform the user if the script is compiled successfully") from validateOutgoingIntegration.ts; instead create or reference an external ticket (e.g., ARCH-2092) to track this work and, if appropriate, add a short one-line comment referencing that ticket (e.g., "Tracked in ARCH-2092") so the implementation contains no TODOs; locate the comment near the webhook/script validation logic in validateOutgoingIntegration (function in validateOutgoingIntegration.ts) and delete the TODO or replace it with the ticket reference.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/meteor/app/integrations/server/lib/validateOutgoingIntegration.ts`:
- Line 191: Remove the inline TODO comment ("// TODO: Webhook Integration Editor
should inform the user if the script is compiled successfully") from
validateOutgoingIntegration.ts; instead create or reference an external ticket
(e.g., ARCH-2092) to track this work and, if appropriate, add a short one-line
comment referencing that ticket (e.g., "Tracked in ARCH-2092") so the
implementation contains no TODOs; locate the comment near the webhook/script
validation logic in validateOutgoingIntegration (function in
validateOutgoingIntegration.ts) and delete the TODO or replace it with the
ticket reference.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 88f9974b-f929-4ce1-98d5-cce86459774c
📒 Files selected for processing (3)
apps/meteor/app/integrations/server/lib/validateOutgoingIntegration.tsapps/meteor/app/integrations/server/methods/incoming/addIncomingIntegration.tsapps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.ts
✅ Files skipped from review due to trivial changes (1)
- apps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/meteor/app/integrations/server/methods/incoming/addIncomingIntegration.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: CodeQL-Build
- GitHub Check: cubic · AI code reviewer
- GitHub Check: CodeQL-Build
🧰 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:
apps/meteor/app/integrations/server/lib/validateOutgoingIntegration.ts
🧠 Learnings (10)
📓 Common learnings
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38974
File: apps/meteor/app/api/server/v1/im.ts:220-221
Timestamp: 2026-02-24T19:09:09.561Z
Learning: In RocketChat/Rocket.Chat OpenAPI migration PRs for apps/meteor/app/api/server/v1 endpoints, maintainers prefer to avoid any logic changes; style-only cleanups (like removing inline comments) may be deferred to follow-ups to keep scope tight.
Learnt from: smirk-dev
Repo: RocketChat/Rocket.Chat PR: 39625
File: apps/meteor/app/api/server/v1/push.ts:85-97
Timestamp: 2026-03-14T14:58:58.834Z
Learning: In RocketChat/Rocket.Chat, the `push.token` POST/DELETE endpoints in `apps/meteor/app/api/server/v1/push.ts` were already migrated to the chained router API pattern on `develop` prior to PR `#39625`. `cleanTokenResult` (which strips `authToken` and returns `PushTokenResult`) and `isPushTokenPOSTProps`/`isPushTokenDELETEProps` validators already exist on `develop`. PR `#39625` only migrates `push.get` and `push.info` to the chained pattern. Do not flag `cleanTokenResult` or `PushTokenResult` as newly introduced behavior-breaking changes when reviewing this PR.
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 0
File: :0-0
Timestamp: 2026-02-24T19:05:56.710Z
Learning: In Rocket.Chat PRs, keep feature PRs free of unrelated lockfile-only dependency bumps; prefer reverting lockfile drift or isolating such bumps into a separate "chore" commit/PR, and always use yarn install --immutable with the Yarn version pinned in package.json via Corepack.
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 0
File: :0-0
Timestamp: 2026-02-24T19:05:56.710Z
Learning: Rocket.Chat repo context: When a workspace manifest on develop already pins a dependency version (e.g., packages/web-ui-registration → "rocket.chat/ui-contexts": "27.0.1"), a lockfile change in a feature PR that upgrades only that dependency’s resolution is considered a manifest-driven sync and can be kept, preferably as a small "chore: sync yarn.lock with manifests" commit.
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38913
File: packages/ddp-client/src/legacy/types/SDKLegacy.ts:34-34
Timestamp: 2026-02-25T20:10:16.987Z
Learning: In the RocketChat/Rocket.Chat monorepo, packages/ddp-client and apps/meteor do not use TypeScript project references. Module augmentations in apps/meteor (e.g., declare module 'rocket.chat/rest-typings') are not visible when compiling packages/ddp-client in isolation, which is why legacy SDK methods that depend on OperationResult types for OpenAPI-migrated endpoints must remain commented out.
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 35995
File: apps/meteor/app/api/server/v1/rooms.ts:1107-1112
Timestamp: 2026-02-23T17:53:18.785Z
Learning: In Rocket.Chat PR reviews, maintain strict scope boundaries—when a PR is focused on a specific endpoint (e.g., rooms.favorite), avoid reviewing or suggesting changes to other endpoints that were incidentally refactored (e.g., rooms.invite) unless explicitly requested by maintainers.
📚 Learning: 2026-03-20T13:51:23.302Z
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 39553
File: apps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.ts:179-181
Timestamp: 2026-03-20T13:51:23.302Z
Learning: In `apps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.ts`, the truthiness guards `...(integration.avatar && { avatar })`, `...(integration.emoji && { emoji })`, `...(integration.alias && { alias })`, and `...(integration.script && { script })` in the `$set` payload of `updateIncomingIntegration` are intentional. Empty-string values for these fields should NOT overwrite the stored value — only truthy values are persisted. Do not flag these as bugs preventing explicit clears.
Applied to files:
apps/meteor/app/integrations/server/lib/validateOutgoingIntegration.ts
📚 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:
apps/meteor/app/integrations/server/lib/validateOutgoingIntegration.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/app/integrations/server/lib/validateOutgoingIntegration.ts
📚 Learning: 2026-03-15T14:31:28.969Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39647
File: apps/meteor/app/api/server/v1/users.ts:710-757
Timestamp: 2026-03-15T14:31:28.969Z
Learning: In RocketChat/Rocket.Chat, the `UserCreateParamsPOST` type in `apps/meteor/app/api/server/v1/users.ts` (migrated from `packages/rest-typings/src/v1/users/UserCreateParamsPOST.ts`) intentionally has `fields: string` (non-optional) and `settings?: IUserSettings` without a corresponding AJV schema entry. This is a pre-existing divergence carried over verbatim from the original rest-typings source (PR `#39647`). Do not flag this type/schema misalignment during the OpenAPI migration review — it is tracked as a separate follow-up fix.
Applied to files:
apps/meteor/app/integrations/server/lib/validateOutgoingIntegration.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:
apps/meteor/app/integrations/server/lib/validateOutgoingIntegration.ts
📚 Learning: 2026-03-18T22:07:19.687Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 38357
File: apps/meteor/app/apps/server/converters/departments.ts:59-70
Timestamp: 2026-03-18T22:07:19.687Z
Learning: In `apps/meteor/app/apps/server/converters/departments.ts`, the `convertAppDepartment` method uses non-null assertions (`!`) on `department.name`, `department.email`, and `department.offlineMessageChannelName` when constructing `newDepartment: ILivechatDepartment`. These fields are optional on `IAppsDepartment`, but the App framework flow guarantees their presence at the call site. Do not flag these non-null assertions as unsafe during code review.
Applied to files:
apps/meteor/app/integrations/server/lib/validateOutgoingIntegration.ts
📚 Learning: 2026-02-25T20:10:16.987Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38913
File: packages/ddp-client/src/legacy/types/SDKLegacy.ts:34-34
Timestamp: 2026-02-25T20:10:16.987Z
Learning: In the RocketChat/Rocket.Chat monorepo, packages/ddp-client and apps/meteor do not use TypeScript project references. Module augmentations in apps/meteor (e.g., declare module 'rocket.chat/rest-typings') are not visible when compiling packages/ddp-client in isolation, which is why legacy SDK methods that depend on OperationResult types for OpenAPI-migrated endpoints must remain commented out.
Applied to files:
apps/meteor/app/integrations/server/lib/validateOutgoingIntegration.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/integrations/server/lib/validateOutgoingIntegration.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/integrations/server/lib/validateOutgoingIntegration.ts
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
pierre-lehnen-rc
approved these changes
Apr 8, 2026
ggazzo
added
the
stat: QA assured
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes
Updates Meteor to 3.4 and bumps all related package versions to their latest stable releases.
Breaking change:
babel-compileris nowdevOnlyIn Meteor 3.4, the
babel-compilerpackage was marked asdevOnly: true(meteor#13797). This means it is completely stripped from production bundles. Our integration script compilation code (Babel.compile()/Babel.getDefaultOptions()) relied on importing{ Babel } from 'meteor/babel-compiler'at runtime, which caused the server to crash on startup:Fix
meteor/babel-compilerwith@babel/corein the 3 integration files that compile user-provided scripts:apps/meteor/app/integrations/server/lib/validateOutgoingIntegration.tsapps/meteor/app/integrations/server/methods/incoming/addIncomingIntegration.tsapps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.tsbabel-compilerfrom.meteor/packages— listing adevOnlypackage explicitly caused Meteor to generate a runtime reference (Package['babel-compiler']) that resolved toundefinedin the production bundle. Theecmascriptpackage already pulls it in as a build dependency.@babel/coreand@babel/preset-envfromdevDependenciestodependencies— these are now needed at runtime to compile integration scripts, so Meteor must include them in the bundle.Steps to test or reproduce
TypeError: Cannot read properties of undefined (reading 'Babel')Task: ARCH-2092
Summary by CodeRabbit