chore: reorganize backend folder structure — Phase 1 (slash commands)#40259
Open
sampaiodiego wants to merge 13 commits into
Open
chore: reorganize backend folder structure — Phase 1 (slash commands)#40259sampaiodiego wants to merge 13 commits into
sampaiodiego wants to merge 13 commits into
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:
WalkthroughIntroduces a migration plan document and tooling to relocate server-side TypeScript modules from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Add three one-time-use Node.js scripts for the backend folder structure migration: - move-module.mjs: moves a directory of files via git mv and updates all relative imports (both within moved files and in external files) - move-batch.mjs: processes a TSV manifest of moves and runs tsc after - verify-no-old-imports.mjs: checks for stale imports to old paths These scripts will be deleted once all migration phases are complete.
Move 17 slash command modules and bot-helpers from app/*/server/ into server/commands/, updating all relative imports. This is the first step of the backend folder structure reorganization. Moved modules: asciiarts, archiveroom, ban, create, help, hide, invite, inviteall, join, kick, leave, me, msg, mute, status, topic, unarchiveroom, bot-helpers.
sampaiodiego
force-pushed
the
new-backend-folder-structure-proposal
branch
from
e58ce17 to
6f0c0d2
Compare
![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: 3
🤖 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/MIGRATION_PLAN.md`:
- Around line 146-166: The document references Bash script names that don't
match the committed Node ESM tools; update all occurrences of move-file.sh,
move-batch.sh, and verify-no-old-imports.sh to the actual committed filenames
move-module.mjs, move-batch.mjs, and verify-no-old-imports.mjs (and replace the
per-file name move-file with move-module where used), and also sync the Phase 7
cleanup list (and any other mentions) to those .mjs names so readers can find
and run move-module.mjs, move-batch.mjs, and verify-no-old-imports.mjs.
In `@apps/meteor/scripts/migration/move-module.mjs`:
- Around line 183-199: The external-import rewrite uses
resolved.startsWith(oldDirAbs) which can false-match siblings (e.g.,
oldDirAbs=".../emoji" matching ".../emoji-custom"); change the boundary check to
require either equality or a path-separator prefix (e.g., resolved === oldDirAbs
|| resolved.startsWith(oldDirAbs + path.sep)) when deciding to rewrite in the
IMPORT_RE replace callback (symbols: IMPORT_RE, resolveImportSpecifier,
externalFile, oldDirAbs, newDirAbs, computeRelativeSpecifier), and make the same
exact boundary check in the earlier discovery logic that builds the affected
external files (findImportsFrom) so the set of files considered for rewriting
matches the files actually rewritten.
- Around line 125-138: The directory-prefix match is too broad and corrupts
sibling imports; update the loop that iterates movedFiles to only treat
directory matches when the target has a path separator after the stripped
extension and to special-case index files: compute oldDir =
oldPath.replace(/\.(ts|tsx|js|jsx)$/, ''), then require
absoluteTarget.startsWith(oldDir + '/') (not oldDir) before rewriting, and if
path.basename(oldDir) === 'index' handle the index-as-dir move by replacing the
oldDir + '/' prefix with the newPath + '/' (otherwise slice using oldDir.length)
so newPath + absoluteTarget.slice(oldDir.length) produces a correct path; apply
these checks around the existing absoluteTarget reassignment using
movedFiles/oldPath/oldDir/newPath/absoluteTarget.
🪄 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: bac4921b-d2f1-4cfc-b6cc-2cbbff969be0
⛔ Files ignored due to path filters (1)
apps/meteor/scripts/migration/phase1-commands.tsvis excluded by!**/*.tsv
📒 Files selected for processing (46)
apps/meteor/MIGRATION_PLAN.mdapps/meteor/scripts/migration/move-batch.mjsapps/meteor/scripts/migration/move-module.mjsapps/meteor/scripts/migration/verify-no-old-imports.mjsapps/meteor/server/commands/archiveroom/index.tsapps/meteor/server/commands/archiveroom/server.tsapps/meteor/server/commands/asciiarts/gimme.tsapps/meteor/server/commands/asciiarts/index.tsapps/meteor/server/commands/asciiarts/lenny.tsapps/meteor/server/commands/asciiarts/shrug.tsapps/meteor/server/commands/asciiarts/tableflip.tsapps/meteor/server/commands/asciiarts/unflip.tsapps/meteor/server/commands/ban/ban.tsapps/meteor/server/commands/ban/index.tsapps/meteor/server/commands/ban/unban.tsapps/meteor/server/commands/bot-helpers/index.tsapps/meteor/server/commands/create/index.tsapps/meteor/server/commands/create/server.tsapps/meteor/server/commands/help/index.tsapps/meteor/server/commands/help/server.tsapps/meteor/server/commands/hide/hide.tsapps/meteor/server/commands/hide/index.tsapps/meteor/server/commands/invite/index.tsapps/meteor/server/commands/invite/server.tsapps/meteor/server/commands/inviteall/index.tsapps/meteor/server/commands/inviteall/server.tsapps/meteor/server/commands/join/index.tsapps/meteor/server/commands/join/server.tsapps/meteor/server/commands/kick/index.tsapps/meteor/server/commands/kick/server.tsapps/meteor/server/commands/leave/index.tsapps/meteor/server/commands/leave/leave.tsapps/meteor/server/commands/me/index.tsapps/meteor/server/commands/me/me.tsapps/meteor/server/commands/msg/index.tsapps/meteor/server/commands/msg/server.tsapps/meteor/server/commands/mute/index.tsapps/meteor/server/commands/mute/mute.tsapps/meteor/server/commands/mute/unmute.tsapps/meteor/server/commands/status/index.tsapps/meteor/server/commands/status/status.tsapps/meteor/server/commands/topic/index.tsapps/meteor/server/commands/topic/topic.tsapps/meteor/server/commands/unarchiveroom/index.tsapps/meteor/server/commands/unarchiveroom/server.tsapps/meteor/server/importPackages.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). (2)
- GitHub Check: 📦 Build Packages
- 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/server/commands/asciiarts/gimme.tsapps/meteor/server/commands/topic/topic.tsapps/meteor/server/commands/asciiarts/tableflip.tsapps/meteor/server/commands/inviteall/server.tsapps/meteor/server/commands/ban/ban.tsapps/meteor/server/commands/create/server.tsapps/meteor/server/commands/archiveroom/server.tsapps/meteor/server/commands/invite/server.tsapps/meteor/server/commands/join/server.tsapps/meteor/server/commands/hide/hide.tsapps/meteor/server/commands/asciiarts/unflip.tsapps/meteor/server/commands/help/server.tsapps/meteor/server/commands/asciiarts/shrug.tsapps/meteor/server/commands/asciiarts/lenny.tsapps/meteor/server/commands/unarchiveroom/server.tsapps/meteor/server/commands/leave/leave.tsapps/meteor/server/commands/mute/mute.tsapps/meteor/server/commands/me/me.tsapps/meteor/server/commands/ban/unban.tsapps/meteor/server/importPackages.tsapps/meteor/server/commands/msg/server.tsapps/meteor/server/commands/mute/unmute.tsapps/meteor/server/commands/kick/server.tsapps/meteor/server/commands/bot-helpers/index.tsapps/meteor/server/commands/status/status.ts
🧠 Learnings (36)
📓 Common learnings
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37654
File: apps/meteor/client/hooks/useAppSlashCommands.ts:32-38
Timestamp: 2025-12-02T22:23:49.593Z
Learning: In apps/meteor/client/hooks/useAppSlashCommands.ts, the `data?.forEach((command) => slashCommands.add(command))` call during render is intentional. The query is configured with `structuralSharing: false` to prevent React Query from keeping stable data references, and `slashCommands.add` is idempotent, so executing on every render is acceptable and ensures the command registry stays current.
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.
📚 Learning: 2025-12-02T22:23:49.593Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37654
File: apps/meteor/client/hooks/useAppSlashCommands.ts:32-38
Timestamp: 2025-12-02T22:23:49.593Z
Learning: In apps/meteor/client/hooks/useAppSlashCommands.ts, the `data?.forEach((command) => slashCommands.add(command))` call during render is intentional. The query is configured with `structuralSharing: false` to prevent React Query from keeping stable data references, and `slashCommands.add` is idempotent, so executing on every render is acceptable and ensures the command registry stays current.
Applied to files:
apps/meteor/server/commands/asciiarts/gimme.tsapps/meteor/server/commands/topic/topic.tsapps/meteor/server/commands/asciiarts/tableflip.tsapps/meteor/server/commands/inviteall/server.tsapps/meteor/server/commands/ban/ban.tsapps/meteor/server/commands/create/server.tsapps/meteor/server/commands/archiveroom/server.tsapps/meteor/server/commands/invite/server.tsapps/meteor/server/commands/join/server.tsapps/meteor/server/commands/hide/hide.tsapps/meteor/server/commands/asciiarts/unflip.tsapps/meteor/server/commands/help/server.tsapps/meteor/server/commands/asciiarts/shrug.tsapps/meteor/server/commands/unarchiveroom/server.tsapps/meteor/server/commands/me/me.tsapps/meteor/server/commands/ban/unban.tsapps/meteor/server/importPackages.tsapps/meteor/server/commands/mute/unmute.tsapps/meteor/server/commands/kick/server.tsapps/meteor/server/commands/status/status.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/server/commands/asciiarts/gimme.tsapps/meteor/server/commands/topic/topic.tsapps/meteor/server/commands/asciiarts/tableflip.tsapps/meteor/server/commands/inviteall/server.tsapps/meteor/server/commands/ban/ban.tsapps/meteor/server/commands/create/server.tsapps/meteor/server/commands/archiveroom/server.tsapps/meteor/server/commands/invite/server.tsapps/meteor/server/commands/join/server.tsapps/meteor/server/commands/hide/hide.tsapps/meteor/server/commands/asciiarts/unflip.tsapps/meteor/server/commands/help/server.tsapps/meteor/server/commands/asciiarts/shrug.tsapps/meteor/server/commands/asciiarts/lenny.tsapps/meteor/server/commands/unarchiveroom/server.tsapps/meteor/server/commands/leave/leave.tsapps/meteor/server/commands/mute/mute.tsapps/meteor/server/commands/me/me.tsapps/meteor/server/commands/ban/unban.tsapps/meteor/server/importPackages.tsapps/meteor/server/commands/msg/server.tsapps/meteor/server/commands/mute/unmute.tsapps/meteor/server/commands/kick/server.tsapps/meteor/server/commands/bot-helpers/index.tsapps/meteor/server/commands/status/status.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/server/commands/asciiarts/gimme.tsapps/meteor/server/commands/topic/topic.tsapps/meteor/server/commands/asciiarts/tableflip.tsapps/meteor/server/commands/inviteall/server.tsapps/meteor/server/commands/ban/ban.tsapps/meteor/server/commands/create/server.tsapps/meteor/server/commands/archiveroom/server.tsapps/meteor/server/commands/invite/server.tsapps/meteor/server/commands/join/server.tsapps/meteor/server/commands/hide/hide.tsapps/meteor/server/commands/asciiarts/unflip.tsapps/meteor/server/commands/help/server.tsapps/meteor/server/commands/asciiarts/shrug.tsapps/meteor/server/commands/asciiarts/lenny.tsapps/meteor/server/commands/unarchiveroom/server.tsapps/meteor/server/commands/leave/leave.tsapps/meteor/server/commands/mute/mute.tsapps/meteor/server/commands/me/me.tsapps/meteor/server/commands/ban/unban.tsapps/meteor/server/importPackages.tsapps/meteor/server/commands/msg/server.tsapps/meteor/server/commands/mute/unmute.tsapps/meteor/server/commands/kick/server.tsapps/meteor/server/commands/bot-helpers/index.tsapps/meteor/server/commands/status/status.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/server/commands/topic/topic.tsapps/meteor/server/commands/inviteall/server.tsapps/meteor/server/commands/create/server.tsapps/meteor/server/commands/invite/server.tsapps/meteor/server/commands/join/server.tsapps/meteor/server/commands/hide/hide.tsapps/meteor/server/commands/help/server.tsapps/meteor/server/commands/me/me.tsapps/meteor/server/importPackages.tsapps/meteor/server/commands/msg/server.tsapps/meteor/server/commands/mute/unmute.tsapps/meteor/server/commands/kick/server.tsapps/meteor/server/commands/bot-helpers/index.tsapps/meteor/server/commands/status/status.tsapps/meteor/MIGRATION_PLAN.md
📚 Learning: 2026-03-09T18:39:21.178Z
Learnt from: Harxhit
Repo: RocketChat/Rocket.Chat PR: 39476
File: apps/meteor/server/methods/addAllUserToRoom.ts:0-0
Timestamp: 2026-03-09T18:39:21.178Z
Learning: In apps/meteor/server/methods/addAllUserToRoom.ts, the implementation uses a single cursor pass (Users.find(userFilter).batchSize(100)) that collects both the full user objects (collectedUsers: IUser[]) and their usernames (usernames: string[]) in one iteration. `beforeAddUserToRoom` is then called once with the full usernames batch (preserving batch-validation semantics), and the subsequent subscription/message processing loop iterates over the same stable `collectedUsers` array — no second DB query is made. This avoids any race condition between validation and processing while preserving the original batch-validation behavior.
Applied to files:
apps/meteor/server/commands/inviteall/server.tsapps/meteor/server/commands/invite/server.tsapps/meteor/server/commands/join/server.tsapps/meteor/server/commands/leave/leave.tsapps/meteor/server/commands/bot-helpers/index.ts
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Applied to files:
apps/meteor/server/commands/inviteall/server.tsapps/meteor/server/commands/create/server.tsapps/meteor/server/commands/archiveroom/server.tsapps/meteor/server/commands/invite/server.tsapps/meteor/server/commands/join/server.tsapps/meteor/server/commands/hide/hide.tsapps/meteor/server/commands/unarchiveroom/server.tsapps/meteor/server/commands/leave/leave.tsapps/meteor/server/commands/bot-helpers/index.ts
📚 Learning: 2026-03-12T10:26:26.697Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39340
File: apps/meteor/app/api/server/v1/im.ts:1349-1398
Timestamp: 2026-03-12T10:26:26.697Z
Learning: In `apps/meteor/app/api/server/v1/im.ts` (PR `#39340`), the `DmEndpoints` type intentionally includes temporary stub entries for `/v1/im.kick`, `/v1/dm.kick`, `/v1/im.leave`, and `/v1/dm.leave` (using `DmKickProps` and `DmLeaveProps`) even though no route handlers exist for them yet. These stubs were added to preserve type compatibility after removing the original `DmLeaveProps` and related files. They are planned for cleanup in a follow-up PR. Do not flag these as missing implementations when reviewing this file until the follow-up is merged.
Applied to files:
apps/meteor/server/commands/inviteall/server.tsapps/meteor/server/commands/create/server.tsapps/meteor/server/commands/invite/server.tsapps/meteor/server/commands/join/server.tsapps/meteor/server/commands/hide/hide.tsapps/meteor/server/commands/help/server.tsapps/meteor/server/commands/leave/leave.tsapps/meteor/server/commands/mute/mute.tsapps/meteor/server/commands/me/me.tsapps/meteor/server/importPackages.tsapps/meteor/server/commands/msg/server.tsapps/meteor/server/commands/mute/unmute.tsapps/meteor/server/commands/kick/server.tsapps/meteor/server/commands/bot-helpers/index.tsapps/meteor/MIGRATION_PLAN.md
📚 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/server/commands/inviteall/server.tsapps/meteor/server/commands/create/server.tsapps/meteor/server/commands/join/server.tsapps/meteor/server/commands/help/server.tsapps/meteor/server/importPackages.tsapps/meteor/server/commands/msg/server.tsapps/meteor/server/commands/kick/server.tsapps/meteor/server/commands/bot-helpers/index.tsapps/meteor/server/commands/status/status.tsapps/meteor/MIGRATION_PLAN.md
📚 Learning: 2026-03-14T14:58:58.834Z
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.
Applied to files:
apps/meteor/server/commands/inviteall/server.tsapps/meteor/server/commands/invite/server.tsapps/meteor/server/commands/kick/server.tsapps/meteor/server/commands/bot-helpers/index.ts
📚 Learning: 2026-03-11T22:04:20.529Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 39545
File: apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts:59-61
Timestamp: 2026-03-11T22:04:20.529Z
Learning: In `apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts`, the `msg.u._id === uid` early-return in the `streamNewMessage` handler is intentional: the "New messages" indicator is designed to notify about messages from other users only. Self-sent messages — including those sent from a different session/device — are always skipped, by design. Do not flag this as a multi-session regression.
Applied to files:
apps/meteor/server/commands/inviteall/server.tsapps/meteor/server/commands/unarchiveroom/server.tsapps/meteor/server/commands/msg/server.ts
📚 Learning: 2025-11-04T16:49:19.107Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.
Applied to files:
apps/meteor/server/commands/inviteall/server.tsapps/meteor/server/commands/archiveroom/server.tsapps/meteor/server/commands/invite/server.tsapps/meteor/server/commands/join/server.tsapps/meteor/server/commands/unarchiveroom/server.tsapps/meteor/server/commands/leave/leave.tsapps/meteor/server/commands/bot-helpers/index.ts
📚 Learning: 2025-11-27T17:56:26.050Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37557
File: apps/meteor/client/views/admin/ABAC/AdminABACRooms.tsx:115-116
Timestamp: 2025-11-27T17:56:26.050Z
Learning: In Rocket.Chat, the GET /v1/abac/rooms endpoint (implemented in ee/packages/abac/src/index.ts) only returns rooms where abacAttributes exists and is not an empty array (query: { abacAttributes: { $exists: true, $ne: [] } }). Therefore, in components consuming this endpoint (like AdminABACRooms.tsx), room.abacAttributes is guaranteed to be defined for all returned rooms, and optional chaining before calling array methods like .join() is sufficient without additional null coalescing.
Applied to files:
apps/meteor/server/commands/inviteall/server.tsapps/meteor/server/commands/join/server.tsapps/meteor/server/commands/hide/hide.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/server/commands/ban/ban.tsapps/meteor/server/commands/create/server.tsapps/meteor/server/commands/invite/server.tsapps/meteor/server/commands/ban/unban.tsapps/meteor/MIGRATION_PLAN.md
📚 Learning: 2025-09-19T15:15:04.642Z
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 36991
File: apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts:219-221
Timestamp: 2025-09-19T15:15:04.642Z
Learning: The Federation_Matrix_homeserver_domain setting in apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts is part of the old federation system and is being deprecated/removed, so configuration issues with this setting should not be flagged for improvement.
Applied to files:
apps/meteor/server/commands/create/server.tsapps/meteor/server/commands/invite/server.tsapps/meteor/server/commands/join/server.tsapps/meteor/server/commands/help/server.tsapps/meteor/server/commands/bot-helpers/index.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : All test files must be created in `apps/meteor/tests/e2e/` directory
Applied to files:
apps/meteor/server/commands/create/server.tsapps/meteor/MIGRATION_PLAN.md
📚 Learning: 2026-04-18T12:32:53.425Z
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 38623
File: apps/meteor/app/lib/server/functions/cleanRoomHistory.ts:146-149
Timestamp: 2026-04-18T12:32:53.425Z
Learning: In `apps/meteor/app/lib/server/functions/cleanRoomHistory.ts` (PR `#38623`), the read-receipt cleanup (both `ReadReceipts.removeByMessageIds` and `ReadReceiptsArchive.removeByMessageIds`) is intentionally only performed in the limited prune path (`limit && selectedMessageIds`). The unlimited/delete-all path (`limit === 0`) deliberately skips cleaning up orphaned read receipts in both hot and cold storage — this is by design. Do not flag this as a bug or missing cleanup in future reviews.
Applied to files:
apps/meteor/server/commands/archiveroom/server.tsapps/meteor/server/commands/unarchiveroom/server.ts
📚 Learning: 2026-03-11T18:15:53.272Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39425
File: apps/meteor/app/api/server/v1/rooms.ts:294-296
Timestamp: 2026-03-11T18:15:53.272Z
Learning: In Rocket.Chat's `rooms.mediaConfirm/:rid/:fileId` endpoint (apps/meteor/app/api/server/v1/rooms.ts), updating `file.name` from `bodyParams.fileName` without updating `file.path` is intentionally safe. The file path is keyed by `_id` (not by filename), so the stored path remains valid regardless of a rename. `file.name` only affects the display name in the message attachment; do not flag this as a path-divergence issue.
Applied to files:
apps/meteor/server/commands/archiveroom/server.tsapps/meteor/server/commands/bot-helpers/index.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings by mapping subscription documents to room IDs, never undefined, even when user has no room subscriptions.
Applied to files:
apps/meteor/server/commands/invite/server.tsapps/meteor/server/commands/join/server.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings (mapping subscription documents to room IDs), never undefined, even when user has no room subscriptions.
Applied to files:
apps/meteor/server/commands/invite/server.tsapps/meteor/server/commands/join/server.ts
📚 Learning: 2025-11-19T12:32:29.696Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37547
File: packages/i18n/src/locales/en.i18n.json:634-634
Timestamp: 2025-11-19T12:32:29.696Z
Learning: Repo: RocketChat/Rocket.Chat
Context: i18n workflow
Learning: In this repository, new translation keys should be added to packages/i18n/src/locales/en.i18n.json only; other locale files are populated via the external translation pipeline and/or fall back to English. Do not request adding the same key to all locale files in future reviews.
Applied to files:
apps/meteor/server/commands/invite/server.tsapps/meteor/server/commands/join/server.ts
📚 Learning: 2025-12-09T20:01:07.355Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37532
File: ee/packages/federation-matrix/src/FederationMatrix.ts:920-927
Timestamp: 2025-12-09T20:01:07.355Z
Learning: In Rocket.Chat's federation invite handling (ee/packages/federation-matrix/src/FederationMatrix.ts), when a user rejects an invite via federationSDK.rejectInvite(), the subscription cleanup happens automatically through an event-driven flow: Matrix emits a leave event back, which is processed by handleLeave() in ee/packages/federation-matrix/src/events/member.ts, and that function calls Room.performUserRemoval() to clean up the subscription. No explicit cleanup is needed in the reject branch of handleInvite() because the leave event handler takes care of it.
<!-- </add_learning>
Applied to files:
apps/meteor/server/commands/invite/server.tsapps/meteor/server/commands/leave/leave.ts
📚 Learning: 2026-04-17T17:38:15.994Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 39858
File: packages/ui-kit/src/interactions/UserInteraction.ts:33-33
Timestamp: 2026-04-17T17:38:15.994Z
Learning: In RocketChat/Rocket.Chat (`packages/ui-kit/src/interactions/UserInteraction.ts`), `ViewSubmitUserInteraction` and `ViewClosedUserInteraction` intentionally do NOT include `rid` when the interaction originates from a **modal** surface. Modals are not scoped to a room, so no room id is available in that context. The `rid?: string` field is optional to support the contextual bar surface (where room context exists) while remaining absent for modals. Do not flag the absence of `rid` in modal viewSubmit/viewClosed interactions as a missing-context bug.
Applied to files:
apps/meteor/server/commands/hide/hide.ts
📚 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/server/commands/asciiarts/shrug.ts
📚 Learning: 2026-03-03T11:11:48.541Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39230
File: apps/meteor/app/api/server/v1/chat.ts:214-222
Timestamp: 2026-03-03T11:11:48.541Z
Learning: In apps/meteor/server/lib/moderation/reportMessage.ts, the reportMessage function validates that description is not empty or whitespace-only with `if (!description.trim())`. When migrating the chat.reportMessage endpoint to OpenAPI, adding minLength validation to the schema preserves this existing behavior.
Applied to files:
apps/meteor/server/commands/msg/server.ts
📚 Learning: 2026-04-17T23:32:07.223Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 38357
File: apps/meteor/app/apps/server/converters/messages.ts:348-352
Timestamp: 2026-04-17T23:32:07.223Z
Learning: In `apps/meteor/app/apps/server/converters/messages.ts`, the `timestamp` handler inside `_convertAttachmentsToApp` uses a non-null assertion (`attachment.ts!`) intentionally. The `ts` property on `MessageAttachment` is optional only to accommodate MessageAttachment creation-time scenarios; by the time `_convertAttachmentsToApp` is called, the message has already been stored and the attachment is guaranteed to have a `ts` value. Do not flag this non-null assertion as unsafe during code review.
Applied to files:
apps/meteor/server/commands/msg/server.ts
📚 Learning: 2026-01-19T18:17:46.433Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38262
File: apps/meteor/client/lib/utils/normalizeMessagePreview/getMessagePreview.ts:24-33
Timestamp: 2026-01-19T18:17:46.433Z
Learning: In the Rocket.Chat repository, usernames (lastMessage.u.username) and display names (lastMessage.u.name) are sanitized/escaped centrally elsewhere in the system, so individual display functions like getMessagePreview do not need to escape these values before interpolating them into strings.
Applied to files:
apps/meteor/server/commands/msg/server.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/server/commands/mute/unmute.tsapps/meteor/scripts/migration/verify-no-old-imports.mjs
📚 Learning: 2025-09-16T13:33:49.237Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36890
File: apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts:21-26
Timestamp: 2025-09-16T13:33:49.237Z
Learning: In Rocket.Chat test files, the im.delete API endpoint accepts either a `roomId` parameter (requiring the actual DM room _id) or a `username` parameter (for the DM partner's username). It does not accept slug-like constructions such as concatenating usernames together.
Applied to files:
apps/meteor/server/commands/kick/server.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/page-objects/**/*.ts : Utilize existing page objects pattern from `apps/meteor/tests/e2e/page-objects/`
Applied to files:
apps/meteor/server/commands/bot-helpers/index.ts
📚 Learning: 2025-12-16T17:29:45.163Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37834
File: apps/meteor/tests/e2e/page-objects/fragments/admin-flextab-emoji.ts:12-22
Timestamp: 2025-12-16T17:29:45.163Z
Learning: In page object files under `apps/meteor/tests/e2e/page-objects/`, always import `expect` from `../../utils/test` (Playwright's async expect), not from Jest. Jest's `expect` has a synchronous signature and will cause TypeScript errors when used with web-first assertions like `toBeVisible()`.
Applied to files:
apps/meteor/scripts/migration/verify-no-old-imports.mjs
📚 Learning: 2026-04-20T17:11:59.452Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 40225
File: apps/meteor/ee/server/apps/communication/endpoints/appLogsHandler.ts:55-71
Timestamp: 2026-04-20T17:11:59.452Z
Learning: In `apps/meteor/ee/server/apps/communication/endpoints/appLogsHandler.ts`, the concern about an empty `?appId=` query param bypassing the truthy check and overriding the path `appId` in the `makeAppLogsQuery` spread is not relevant. The AJV query schema (`isAppLogsProps`) validates and rejects invalid/empty `appId` values before the action handler is reached, making the in-handler guard sufficient as-is. Do not flag this pattern as a vulnerability in future reviews of this file.
Applied to files:
apps/meteor/scripts/migration/verify-no-old-imports.mjs
📚 Learning: 2026-03-16T21:50:42.118Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39676
File: .changeset/migrate-users-register-openapi.md:3-3
Timestamp: 2026-03-16T21:50:42.118Z
Learning: In RocketChat/Rocket.Chat OpenAPI migration PRs, removing endpoint types and validators from `rocket.chat/rest-typings` (e.g., `UserRegisterParamsPOST`, `/v1/users.register` entry) is the *required* migration pattern per RocketChat/Rocket.Chat-Open-API#150 Rule 7 ("No More rest-typings or Manual Typings"). The endpoint type is re-exposed via a module augmentation `.d.ts` file in the consuming package (e.g., `packages/web-ui-registration/src/users-register.d.ts`). This is NOT a breaking change — the correct changeset bump for `rocket.chat/rest-typings` in this scenario is `minor`, not `major`. Do not flag this as a breaking change during OpenAPI migration reviews.
Applied to files:
apps/meteor/scripts/migration/verify-no-old-imports.mjsapps/meteor/MIGRATION_PLAN.md
📚 Learning: 2026-03-16T11:57:17.987Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 39657
File: apps/meteor/tests/end-to-end/apps/app-resolve-visitor.ts:43-45
Timestamp: 2026-03-16T11:57:17.987Z
Learning: In `apps/meteor/tests/end-to-end/apps/app-resolve-visitor.ts`, `externalIds` lookups are namespaced per `app.id`. Because `cleanupApps()` is called before each test run and a fresh app is installed (giving a new unique `app.id`), fixed `userId` strings like `'nonexistent-id'` in null-path tests are safe and cannot collide with stale visitor records from previous runs. Do not flag these as needing unique/random values.
Applied to files:
apps/meteor/scripts/migration/verify-no-old-imports.mjs
📚 Learning: 2026-03-20T13:52:29.575Z
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 39553
File: apps/meteor/app/api/server/v1/stats.ts:98-117
Timestamp: 2026-03-20T13:52:29.575Z
Learning: In `apps/meteor/app/api/server/v1/stats.ts`, the `statistics.telemetry` POST endpoint intentionally has no `body` AJV schema in its route options. The proper request body shape (a `params` array of telemetry event objects) has not been formally defined yet, so body validation is deferred to a follow-up. Do not flag the missing body schema for this endpoint during OpenAPI migration reviews.
Applied to files:
apps/meteor/MIGRATION_PLAN.md
📚 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/MIGRATION_PLAN.md
🪛 LanguageTool
apps/meteor/MIGRATION_PLAN.md
[uncategorized] ~432-~432: The official name of this software platform is spelled with a capital “H”.
Context: ...uth-providers/drupal.ts | |app/github/server/ |server...
(GITHUB)
[uncategorized] ~432-~432: The official name of this software platform is spelled with a capital “H”.
Context: .../github/server/ |server/lib/auth-providers/github.ts | |app/github-enterpri...
(GITHUB)
[uncategorized] ~437-~437: The official name of this content management system is spelled with a capital “P”.
Context: ...uth-providers/iframe.ts | |app/wordpress/server/ |server/li...
(WORDPRESS)
[uncategorized] ~437-~437: The official name of this content management system is spelled with a capital “P”.
Context: .../wordpress/server/ |server/lib/auth-providers/wordpress.ts | |app/lib/server/oauth/*...
(WORDPRESS)
[style] ~612-~612: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ds//` but continue to work. - No new abstractions. No port interfaces,...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~613-~613: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ection, no new TypeScript patterns. - **No reorganization of existing server/ fi...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 markdownlint-cli2 (0.22.0)
apps/meteor/MIGRATION_PLAN.md
[warning] 11-11: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 23-23: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (24)
apps/meteor/server/commands/asciiarts/gimme.ts (1)
3-4: Import rewiring looks correct for the new folder depth.
The updated paths resolve to the expectedapp/...modules and are consistent with the migration pattern.apps/meteor/server/commands/topic/topic.ts (1)
3-5: Path updates are consistent and safe.
No functional or typing concerns in this import-only change.apps/meteor/server/commands/join/server.ts (1)
6-8: LGTM on the migrated imports.
The new relative paths align with the relocatedserver/commands/joinmodule structure.apps/meteor/server/commands/asciiarts/tableflip.ts (1)
3-4: Good import migration update.
The rewritten paths are consistent with the new command location and existing module exports.apps/meteor/server/commands/me/me.ts (1)
3-4: Import path changes look correct.
This is a clean relocation update with no additional concerns.apps/meteor/server/commands/help/server.ts (1)
5-7: This import rewrite is consistent with Phase 1 migration.
No correctness or integration issues identified in the changed lines.apps/meteor/server/commands/inviteall/server.ts (1)
12-18: Comprehensive import migration here is well-aligned.
All rewritten paths follow the new structure and keep behavior unchanged.apps/meteor/server/commands/invite/server.ts (1)
8-12: Import adjustments are correct and consistent.
The new paths map to valid modules and preserve current command behavior.apps/meteor/server/commands/ban/unban.ts (1)
5-9: LGTM! Import paths correctly updated.The import path updates are consistent with the migration from
app/*/server/toserver/commands/structure. All paths correctly navigate from the new location to their target modules.apps/meteor/server/commands/asciiarts/shrug.ts (1)
3-4: LGTM! Import paths correctly updated.The import path updates correctly navigate from
server/commands/asciiarts/to the target modules inapp/lib/andapp/utils/. The refactoring is clean and consistent with the migration plan.apps/meteor/server/commands/asciiarts/unflip.ts (1)
3-4: LGTM! Import paths correctly updated.The import path updates are consistent with the migration pattern and match the updates in other ASCII art commands.
apps/meteor/server/commands/unarchiveroom/server.ts (1)
8-13: LGTM! Import paths correctly updated.All six import path updates correctly reference the relocated modules, maintaining the proper navigation from
server/commands/unarchiveroom/to their target locations inserver/lib/andapp/*/server/.apps/meteor/server/commands/status/status.ts (1)
5-8: LGTM! Import paths correctly updated.The import path updates are consistent with the migration structure, correctly navigating from the new command location to the target modules.
apps/meteor/server/commands/create/server.ts (1)
5-9: LGTM! Import paths correctly updated.All import path updates correctly reference the relocated modules. The paths are consistent with the migration pattern across all command files.
apps/meteor/server/commands/leave/leave.ts (1)
6-9: LGTM! Import paths correctly updated.The import path updates correctly navigate from the new
server/commands/leave/location to the target modules, maintaining consistency with the overall migration structure.apps/meteor/server/commands/hide/hide.ts (1)
5-8: LGTM! Import paths correctly updated.The import path updates are consistent with the migration structure. Note that
hideRoomMethodimports fromserver/methods/(line 6) while other imports follow theapp/*/server/pattern, which appears to be the intended structure for this module's new location.apps/meteor/server/commands/bot-helpers/index.ts (1)
8-13: Import relocation looks correct and consistent.The updated paths align with the new
server/commandslayout and preserve existing behavior.apps/meteor/server/commands/mute/mute.ts (1)
5-8: LGTM — import updates are valid for the new structure.No behavior change introduced in this segment; the module targets are consistent with the migration.
apps/meteor/server/commands/kick/server.ts (1)
6-10: Good import-path migration for/kick.These rewired imports match the new command-module layout and remain compatible with existing exports.
apps/meteor/server/commands/mute/unmute.ts (1)
5-8: Import rewiring is clean and correct.The new paths are consistent with the Phase 1 migration and do not introduce behavior changes.
apps/meteor/server/commands/archiveroom/server.ts (1)
8-13: Looks good — imports are correctly redirected.This segment is consistent with the responsibility-first folder migration and preserves command behavior.
apps/meteor/server/commands/asciiarts/lenny.ts (1)
3-4: Import updates are correct for the relocated ascii command.No issues found in this changed segment.
apps/meteor/server/commands/ban/ban.ts (1)
5-9: Path migration for/banimports is solid.The updated import sources align with the new structure and keep existing command flow intact.
apps/meteor/server/commands/msg/server.ts (1)
6-10: Import rewiring for/msgis correct.This segment matches the new module boundaries and preserves existing behavior.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #40259 +/- ##
===========================================
- Coverage 69.83% 69.64% -0.19%
===========================================
Files 3296 3321 +25
Lines 119173 119679 +506
Branches 21480 21639 +159
===========================================
+ Hits 83219 83346 +127
- Misses 32645 32996 +351
- Partials 3309 3337 +28
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/scripts/migration/move-batch.mjs`:
- Line 16: The current use of execSync in move-batch.mjs builds a shell command
string with TSV-controlled paths, which allows shell interpolation; replace the
shell-invoking execSync call with an argv-based API (e.g., execFileSync or
spawnSync) so each path is passed as a separate argument rather than
interpolated into a shell string. Locate the execSync usage in move-batch.mjs
(the command that includes TSV-derived variables) and change it to call
execFileSync/child_process.spawnSync with the program as the first arg and an
array of path/flag arguments, ensuring you do not enable shell:true and properly
escape or validate path inputs before calling.
🪄 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: 147ae1de-3e32-4068-ba63-5a7758dad5d2
📒 Files selected for processing (20)
apps/meteor/scripts/migration/move-batch.mjsapps/meteor/scripts/migration/move-module.mjsapps/meteor/scripts/migration/verify-no-old-imports.mjsapps/meteor/server/commands/archiveroom/server.tsapps/meteor/server/commands/ban/ban.tsapps/meteor/server/commands/ban/unban.tsapps/meteor/server/commands/bot-helpers/index.tsapps/meteor/server/commands/create/server.tsapps/meteor/server/commands/help/server.tsapps/meteor/server/commands/hide/hide.tsapps/meteor/server/commands/invite/server.tsapps/meteor/server/commands/inviteall/server.tsapps/meteor/server/commands/join/server.tsapps/meteor/server/commands/kick/server.tsapps/meteor/server/commands/leave/leave.tsapps/meteor/server/commands/msg/server.tsapps/meteor/server/commands/mute/mute.tsapps/meteor/server/commands/mute/unmute.tsapps/meteor/server/commands/status/status.tsapps/meteor/server/commands/unarchiveroom/server.ts
✅ Files skipped from review due to trivial changes (14)
- apps/meteor/server/commands/help/server.ts
- apps/meteor/server/commands/leave/leave.ts
- apps/meteor/server/commands/hide/hide.ts
- apps/meteor/server/commands/inviteall/server.ts
- apps/meteor/server/commands/invite/server.ts
- apps/meteor/server/commands/archiveroom/server.ts
- apps/meteor/server/commands/unarchiveroom/server.ts
- apps/meteor/server/commands/join/server.ts
- apps/meteor/server/commands/ban/unban.ts
- apps/meteor/server/commands/status/status.ts
- apps/meteor/server/commands/mute/mute.ts
- apps/meteor/server/commands/mute/unmute.ts
- apps/meteor/server/commands/msg/server.ts
- apps/meteor/server/commands/kick/server.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/meteor/server/commands/ban/ban.ts
- apps/meteor/scripts/migration/verify-no-old-imports.mjs
- apps/meteor/server/commands/create/server.ts
- apps/meteor/scripts/migration/move-module.mjs
- apps/meteor/server/commands/bot-helpers/index.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: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
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.
- Sync MIGRATION_PLAN.md script names to actual .mjs filenames - Add path.sep boundary to prefix matching in move-module.mjs to prevent mis-matching siblings (e.g., emoji vs emoji-custom) - Use execFileSync in move-batch.mjs to avoid shell interpolation
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/MIGRATION_PLAN.md`:
- Around line 203-220: The Phase 2 header "Phase 2: External Bridges (5 folders
→ `server/bridges/`)" is inconsistent with the mapping table that lists seven
source folders; update the header to reflect the correct count (change "5
folders" to "7 folders") or remove the explicit folder count so it can't fall
out of sync, and leave the rest of the table unchanged; target the "Phase 2:
External Bridges (5 folders → `server/bridges/`)" heading text in
MIGRATION_PLAN.md when making the edit.
- Around line 11-13: Add language identifiers to the fenced code blocks in
MIGRATION_PLAN.md: replace the triple-backtick blocks surrounding the snippet
that starts with "server/<responsibility>/<domain>/<file>" and the larger block
that begins with "apps/meteor/server/" (which includes "importPackages.ts") with
language-tagged fences (e.g., ```text) so markdownlint MD040 is satisfied; also
scan the rest of the file (lines shown as 23-130 in the review) and update any
other plain ``` fences to ```text where they contain plain text examples.
- Around line 169-197: The doc states Phase 1 only consolidates slash-commands
but the PR moves bot-helpers in Phase 1 too; update MIGRATION_PLAN.md to remove
this inconsistency by either adding bot-helpers to the Phase 1 table (e.g., add
an entry mapping its original folder to server/commands/bot-helpers or
server/helpers as appropriate) and updating the "Import updates" notes to
mention bot-helpers, or keep bot-helpers deferred and remove its mention from
the Phase 1 move in this PR; also remove or reconcile the conflicting deferred
mention of "bot-helpers" in the later phase section so both sections match.
🪄 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: 4d0aca6e-fb5e-46f9-8c5f-cbd4f71419cb
📒 Files selected for processing (3)
apps/meteor/MIGRATION_PLAN.mdapps/meteor/scripts/migration/move-batch.mjsapps/meteor/scripts/migration/move-module.mjs
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/meteor/scripts/migration/move-batch.mjs
- apps/meteor/scripts/migration/move-module.mjs
📜 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: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
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.
📚 Learning: 2026-03-12T10:26:26.697Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39340
File: apps/meteor/app/api/server/v1/im.ts:1349-1398
Timestamp: 2026-03-12T10:26:26.697Z
Learning: In `apps/meteor/app/api/server/v1/im.ts` (PR `#39340`), the `DmEndpoints` type intentionally includes temporary stub entries for `/v1/im.kick`, `/v1/dm.kick`, `/v1/im.leave`, and `/v1/dm.leave` (using `DmKickProps` and `DmLeaveProps`) even though no route handlers exist for them yet. These stubs were added to preserve type compatibility after removing the original `DmLeaveProps` and related files. They are planned for cleanup in a follow-up PR. Do not flag these as missing implementations when reviewing this file until the follow-up is merged.
Applied to files:
apps/meteor/MIGRATION_PLAN.md
📚 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/MIGRATION_PLAN.md
📚 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/MIGRATION_PLAN.md
📚 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/MIGRATION_PLAN.md
📚 Learning: 2026-03-16T21:50:42.118Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39676
File: .changeset/migrate-users-register-openapi.md:3-3
Timestamp: 2026-03-16T21:50:42.118Z
Learning: In RocketChat/Rocket.Chat OpenAPI migration PRs, removing endpoint types and validators from `rocket.chat/rest-typings` (e.g., `UserRegisterParamsPOST`, `/v1/users.register` entry) is the *required* migration pattern per RocketChat/Rocket.Chat-Open-API#150 Rule 7 ("No More rest-typings or Manual Typings"). The endpoint type is re-exposed via a module augmentation `.d.ts` file in the consuming package (e.g., `packages/web-ui-registration/src/users-register.d.ts`). This is NOT a breaking change — the correct changeset bump for `rocket.chat/rest-typings` in this scenario is `minor`, not `major`. Do not flag this as a breaking change during OpenAPI migration reviews.
Applied to files:
apps/meteor/MIGRATION_PLAN.md
📚 Learning: 2026-03-20T13:52:29.575Z
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 39553
File: apps/meteor/app/api/server/v1/stats.ts:98-117
Timestamp: 2026-03-20T13:52:29.575Z
Learning: In `apps/meteor/app/api/server/v1/stats.ts`, the `statistics.telemetry` POST endpoint intentionally has no `body` AJV schema in its route options. The proper request body shape (a `params` array of telemetry event objects) has not been formally defined yet, so body validation is deferred to a follow-up. Do not flag the missing body schema for this endpoint during OpenAPI migration reviews.
Applied to files:
apps/meteor/MIGRATION_PLAN.md
📚 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/MIGRATION_PLAN.md
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : All test files must be created in `apps/meteor/tests/e2e/` directory
Applied to files:
apps/meteor/MIGRATION_PLAN.md
📚 Learning: 2025-09-19T15:15:04.642Z
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 36991
File: apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts:219-221
Timestamp: 2025-09-19T15:15:04.642Z
Learning: The Federation_Matrix_homeserver_domain setting in apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts is part of the old federation system and is being deprecated/removed, so configuration issues with this setting should not be flagged for improvement.
Applied to files:
apps/meteor/MIGRATION_PLAN.md
🪛 LanguageTool
apps/meteor/MIGRATION_PLAN.md
[uncategorized] ~431-~431: The official name of this software platform is spelled with a capital “H”.
Context: ...uth-providers/drupal.ts | |app/github/server/ |server...
(GITHUB)
[uncategorized] ~431-~431: The official name of this software platform is spelled with a capital “H”.
Context: .../github/server/ |server/lib/auth-providers/github.ts | |app/github-enterpri...
(GITHUB)
[uncategorized] ~436-~436: The official name of this content management system is spelled with a capital “P”.
Context: ...uth-providers/iframe.ts | |app/wordpress/server/ |server/li...
(WORDPRESS)
[uncategorized] ~436-~436: The official name of this content management system is spelled with a capital “P”.
Context: .../wordpress/server/ |server/lib/auth-providers/wordpress.ts | |app/lib/server/oauth/*...
(WORDPRESS)
[style] ~611-~611: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ds//` but continue to work. - No new abstractions. No port interfaces,...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~612-~612: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ection, no new TypeScript patterns. - **No reorganization of existing server/ fi...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 markdownlint-cli2 (0.22.0)
apps/meteor/MIGRATION_PLAN.md
[warning] 11-11: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 23-23: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
Comment on lines
+11
to
+13
| ``` | ||
| server/<responsibility>/<domain>/<file> | ||
| ``` |
Contributor
There was a problem hiding this comment.
Add language identifiers to fenced code blocks.
Both fenced blocks are missing a language tag, which will keep triggering markdownlint MD040.
📝 Proposed doc fix
-```
+```text
server/<responsibility>/<domain>/<file>...
- +text
apps/meteor/server/
│
│ ── NEW (from app/*/server/) ──────────────────────────────
...
└── importPackages.ts # EXISTING — updated imports
Also applies to: 23-130
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 11-11: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/meteor/MIGRATION_PLAN.md` around lines 11 - 13, Add language identifiers
to the fenced code blocks in MIGRATION_PLAN.md: replace the triple-backtick
blocks surrounding the snippet that starts with
"server/<responsibility>/<domain>/<file>" and the larger block that begins with
"apps/meteor/server/" (which includes "importPackages.ts") with language-tagged
fences (e.g., ```text) so markdownlint MD040 is satisfied; also scan the rest of
the file (lines shown as 23-130 in the review) and update any other plain ```
fences to ```text where they contain plain text examples.
Comment on lines
+203
to
+220
| ### Phase 2: External Bridges (5 folders → `server/bridges/`) | ||
|
|
||
| **Goal**: Move isolated bridge code that has no inbound importers. | ||
|
|
||
| **Risk**: Low. Bridges are leaf nodes — they import from the core but nothing imports from them. | ||
|
|
||
| **Scope**: ~48 files | ||
|
|
||
| | Source | Destination | | ||
| | ------------------------------ | ---------------------------------------- | | ||
| | `app/apps/server/bridges/` | `server/bridges/apps-engine/bridges/` | | ||
| | `app/apps/server/converters/` | `server/bridges/apps-engine/converters/` | | ||
| | `app/irc/server/` | `server/bridges/irc/` | | ||
| | `app/slackbridge/server/` | `server/bridges/slack/` | | ||
| | `app/smarsh-connector/server/` | `server/bridges/smarsh/` | | ||
| | `app/webdav/server/` | `server/bridges/webdav/` | | ||
| | `app/nextcloud/server/` | `server/bridges/nextcloud/` | | ||
|
|
Contributor
There was a problem hiding this comment.
Phase 2 title count does not match the mapping table.
Line 203 says “5 folders,” but Lines 213-219 list 7 source folders. Please align the heading count with the actual scope.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/meteor/MIGRATION_PLAN.md` around lines 203 - 220, The Phase 2 header
"Phase 2: External Bridges (5 folders → `server/bridges/`)" is inconsistent with
the mapping table that lists seven source folders; update the header to reflect
the correct count (change "5 folders" to "7 folders") or remove the explicit
folder count so it can't fall out of sync, and leave the rest of the table
unchanged; target the "Phase 2: External Bridges (5 folders →
`server/bridges/`)" heading text in MIGRATION_PLAN.md when making the edit.
🔴 Layne — 16 finding(s)Found 16 issue(s): 16 high. |
…rs to server/lib The Phase 1 target folder is renamed to `slashcommands/` to match the existing app/slashcommands-* naming and make the folder's purpose obvious. `bot-helpers/` is pulled out into `server/lib/bot-helpers/` since it isn't a slash command — it got bundled into the Phase 1 commit by mistake. Also updates MIGRATION_PLAN.md and the disposable Phase 0/1 migration scripts to reflect the new target paths. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/meteor/scripts/migration/move-module.mjs (2)
83-85: Silent fallback for unresolved relative imports.When none of the candidate paths exist,
resolveImportSpecifierreturns the bareresolvedpath anyway. InupdateImportsInFile(line 122) that means a stale/typo'd relative import inside a moved file will still get blindly recomputed against the new location and potentially rewritten to something that also doesn't exist, without any warning. Given this tool will drive many more phases, a one-lineconsole.warnon the "fell through all candidates" branch would make truly broken imports visible earlier instead of producing silently-wrong rewrites.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/scripts/migration/move-module.mjs` around lines 83 - 85, The resolveImportSpecifier function currently returns the computed `resolved` path even when none of the candidate paths exist, which leads updateImportsInFile to silently rewrite imports to non-existent targets; modify resolveImportSpecifier so that in the "fell through all candidates" branch it emits a warning (e.g., console.warn) that includes the original import specifier, the computed `resolved` path, and the source file path, so broken or typo'd relative imports are visible when updateImportsInFile runs; keep returning `resolved` for fallback behavior but ensure the warning gives enough context to triage the bad rewrite.
231-237: Consider broadeningsearchDirsfor later phases.The external-import scan only walks
app/,server/,ee/,lib/underapps/meteor/. That's sufficient for Phase 1 (slash commands are server-only leaf nodes), but later phases move code that may be referenced fromclient/,tests/,imports/, or top-levelpackages/. When those phases run, unreferenced-but-real imports from those trees will be silently left pointing at old paths untilverify-no-old-imports.mjscatches them. Worth either (a) expandingsearchDirsnow or (b) leaving a note on the script to widen the list per phase.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/scripts/migration/move-module.mjs` around lines 231 - 237, The searchDirs array used in move-module.mjs (variable searchDirs) only includes 'app','server','ee','lib' so externalFiles (populated via getAllFiles and pushed into externalFiles) misses imports in client/, tests/, imports/, and top-level packages/, causing stale imports until verify-no-old-imports.mjs finds them; update searchDirs to include 'client', 'tests', 'imports', and the top-level 'packages' tree (or add a clear TODO comment near the searchDirs declaration explaining to expand it per phase) so getAllFiles will scan those directories and externalFiles will contain those references.
🤖 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/MIGRATION_PLAN.md`:
- Around line 49-67: Update the Phase 1 documentation in MIGRATION_PLAN.md to
match the actual shipped subdirectory layout: replace the flat listing under
slashcommands/ (the files like ban.ts, create.ts, help.ts, etc.) with the
per-command subdirectories pattern (e.g., ban/{ban.ts,unban.ts,index.ts},
create/{server.ts,index.ts}, etc.) and ensure the Phase 1 destination table
(lines ~177-195) is updated similarly; reference the actual import style shown
in importPackages.ts (import './slashcommands/ban' resolving to ban/index.ts)
when describing the folder structure so the plan aligns with what Phase 1
delivered.
- Line 179: The table row contains a typo: the source token
`slashcommand-asciiarts` is missing the trailing "s"; update that token to
`slashcommands-asciiarts` in the migration table row so it matches the other
`slashcommands-*` entries and the destination `server/slashcommands/asciiarts/`.
---
Nitpick comments:
In `@apps/meteor/scripts/migration/move-module.mjs`:
- Around line 83-85: The resolveImportSpecifier function currently returns the
computed `resolved` path even when none of the candidate paths exist, which
leads updateImportsInFile to silently rewrite imports to non-existent targets;
modify resolveImportSpecifier so that in the "fell through all candidates"
branch it emits a warning (e.g., console.warn) that includes the original import
specifier, the computed `resolved` path, and the source file path, so broken or
typo'd relative imports are visible when updateImportsInFile runs; keep
returning `resolved` for fallback behavior but ensure the warning gives enough
context to triage the bad rewrite.
- Around line 231-237: The searchDirs array used in move-module.mjs (variable
searchDirs) only includes 'app','server','ee','lib' so externalFiles (populated
via getAllFiles and pushed into externalFiles) misses imports in client/,
tests/, imports/, and top-level packages/, causing stale imports until
verify-no-old-imports.mjs finds them; update searchDirs to include 'client',
'tests', 'imports', and the top-level 'packages' tree (or add a clear TODO
comment near the searchDirs declaration explaining to expand it per phase) so
getAllFiles will scan those directories and externalFiles will contain those
references.
🪄 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: 0073741f-aa07-4c4e-ac7b-8f3e6fd11bca
⛔ Files ignored due to path filters (1)
apps/meteor/scripts/migration/phase1-commands.tsvis excluded by!**/*.tsv
📒 Files selected for processing (45)
apps/meteor/MIGRATION_PLAN.mdapps/meteor/scripts/migration/move-batch.mjsapps/meteor/scripts/migration/move-module.mjsapps/meteor/server/importPackages.tsapps/meteor/server/lib/bot-helpers/index.tsapps/meteor/server/slashcommands/archiveroom/index.tsapps/meteor/server/slashcommands/archiveroom/server.tsapps/meteor/server/slashcommands/asciiarts/gimme.tsapps/meteor/server/slashcommands/asciiarts/index.tsapps/meteor/server/slashcommands/asciiarts/lenny.tsapps/meteor/server/slashcommands/asciiarts/shrug.tsapps/meteor/server/slashcommands/asciiarts/tableflip.tsapps/meteor/server/slashcommands/asciiarts/unflip.tsapps/meteor/server/slashcommands/ban/ban.tsapps/meteor/server/slashcommands/ban/index.tsapps/meteor/server/slashcommands/ban/unban.tsapps/meteor/server/slashcommands/create/index.tsapps/meteor/server/slashcommands/create/server.tsapps/meteor/server/slashcommands/help/index.tsapps/meteor/server/slashcommands/help/server.tsapps/meteor/server/slashcommands/hide/hide.tsapps/meteor/server/slashcommands/hide/index.tsapps/meteor/server/slashcommands/invite/index.tsapps/meteor/server/slashcommands/invite/server.tsapps/meteor/server/slashcommands/inviteall/index.tsapps/meteor/server/slashcommands/inviteall/server.tsapps/meteor/server/slashcommands/join/index.tsapps/meteor/server/slashcommands/join/server.tsapps/meteor/server/slashcommands/kick/index.tsapps/meteor/server/slashcommands/kick/server.tsapps/meteor/server/slashcommands/leave/index.tsapps/meteor/server/slashcommands/leave/leave.tsapps/meteor/server/slashcommands/me/index.tsapps/meteor/server/slashcommands/me/me.tsapps/meteor/server/slashcommands/msg/index.tsapps/meteor/server/slashcommands/msg/server.tsapps/meteor/server/slashcommands/mute/index.tsapps/meteor/server/slashcommands/mute/mute.tsapps/meteor/server/slashcommands/mute/unmute.tsapps/meteor/server/slashcommands/status/index.tsapps/meteor/server/slashcommands/status/status.tsapps/meteor/server/slashcommands/topic/index.tsapps/meteor/server/slashcommands/topic/topic.tsapps/meteor/server/slashcommands/unarchiveroom/index.tsapps/meteor/server/slashcommands/unarchiveroom/server.ts
✅ Files skipped from review due to trivial changes (19)
- apps/meteor/server/slashcommands/me/me.ts
- apps/meteor/server/slashcommands/asciiarts/gimme.ts
- apps/meteor/server/slashcommands/help/server.ts
- apps/meteor/server/slashcommands/asciiarts/unflip.ts
- apps/meteor/server/slashcommands/asciiarts/tableflip.ts
- apps/meteor/server/slashcommands/mute/mute.ts
- apps/meteor/server/slashcommands/asciiarts/lenny.ts
- apps/meteor/server/slashcommands/status/status.ts
- apps/meteor/server/slashcommands/ban/ban.ts
- apps/meteor/server/slashcommands/ban/unban.ts
- apps/meteor/server/slashcommands/hide/hide.ts
- apps/meteor/server/slashcommands/invite/server.ts
- apps/meteor/server/slashcommands/kick/server.ts
- apps/meteor/server/slashcommands/mute/unmute.ts
- apps/meteor/server/slashcommands/topic/topic.ts
- apps/meteor/server/slashcommands/inviteall/server.ts
- apps/meteor/server/slashcommands/unarchiveroom/server.ts
- apps/meteor/server/slashcommands/asciiarts/shrug.ts
- apps/meteor/server/slashcommands/join/server.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/meteor/scripts/migration/move-batch.mjs
📜 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). (2)
- 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/server/slashcommands/create/server.tsapps/meteor/server/slashcommands/msg/server.tsapps/meteor/server/slashcommands/leave/leave.tsapps/meteor/server/slashcommands/archiveroom/server.tsapps/meteor/server/importPackages.tsapps/meteor/server/lib/bot-helpers/index.ts
🧠 Learnings (28)
📓 Common learnings
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: 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: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37654
File: apps/meteor/client/hooks/useAppSlashCommands.ts:32-38
Timestamp: 2025-12-02T22:23:49.593Z
Learning: In apps/meteor/client/hooks/useAppSlashCommands.ts, the `data?.forEach((command) => slashCommands.add(command))` call during render is intentional. The query is configured with `structuralSharing: false` to prevent React Query from keeping stable data references, and `slashCommands.add` is idempotent, so executing on every render is acceptable and ensures the command registry stays current.
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 38071
File: apps/meteor/app/apps/server/bridges/listeners.ts:257-271
Timestamp: 2026-01-15T22:03:35.587Z
Learning: In the file upload pipeline (apps/meteor/app/apps/server/bridges/listeners.ts), temporary files are created by the server in the same filesystem, so symlinks between temp files are safe and don't require cross-filesystem fallbacks.
📚 Learning: 2025-12-02T22:23:49.593Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37654
File: apps/meteor/client/hooks/useAppSlashCommands.ts:32-38
Timestamp: 2025-12-02T22:23:49.593Z
Learning: In apps/meteor/client/hooks/useAppSlashCommands.ts, the `data?.forEach((command) => slashCommands.add(command))` call during render is intentional. The query is configured with `structuralSharing: false` to prevent React Query from keeping stable data references, and `slashCommands.add` is idempotent, so executing on every render is acceptable and ensures the command registry stays current.
Applied to files:
apps/meteor/server/slashcommands/create/server.tsapps/meteor/server/slashcommands/msg/server.tsapps/meteor/server/slashcommands/archiveroom/server.tsapps/meteor/server/importPackages.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/server/slashcommands/create/server.tsapps/meteor/server/slashcommands/msg/server.tsapps/meteor/server/slashcommands/archiveroom/server.tsapps/meteor/server/importPackages.tsapps/meteor/server/lib/bot-helpers/index.tsapps/meteor/MIGRATION_PLAN.md
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Applied to files:
apps/meteor/server/slashcommands/create/server.tsapps/meteor/server/slashcommands/leave/leave.tsapps/meteor/server/slashcommands/archiveroom/server.tsapps/meteor/server/lib/bot-helpers/index.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/server/slashcommands/create/server.tsapps/meteor/server/slashcommands/msg/server.tsapps/meteor/server/slashcommands/leave/leave.tsapps/meteor/server/slashcommands/archiveroom/server.tsapps/meteor/server/importPackages.tsapps/meteor/server/lib/bot-helpers/index.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/server/slashcommands/create/server.tsapps/meteor/server/slashcommands/msg/server.tsapps/meteor/server/slashcommands/leave/leave.tsapps/meteor/server/slashcommands/archiveroom/server.tsapps/meteor/server/importPackages.tsapps/meteor/server/lib/bot-helpers/index.ts
📚 Learning: 2026-03-12T10:26:26.697Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39340
File: apps/meteor/app/api/server/v1/im.ts:1349-1398
Timestamp: 2026-03-12T10:26:26.697Z
Learning: In `apps/meteor/app/api/server/v1/im.ts` (PR `#39340`), the `DmEndpoints` type intentionally includes temporary stub entries for `/v1/im.kick`, `/v1/dm.kick`, `/v1/im.leave`, and `/v1/dm.leave` (using `DmKickProps` and `DmLeaveProps`) even though no route handlers exist for them yet. These stubs were added to preserve type compatibility after removing the original `DmLeaveProps` and related files. They are planned for cleanup in a follow-up PR. Do not flag these as missing implementations when reviewing this file until the follow-up is merged.
Applied to files:
apps/meteor/server/slashcommands/msg/server.tsapps/meteor/server/slashcommands/leave/leave.tsapps/meteor/server/importPackages.tsapps/meteor/server/lib/bot-helpers/index.tsapps/meteor/MIGRATION_PLAN.md
📚 Learning: 2026-03-11T22:04:20.529Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 39545
File: apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts:59-61
Timestamp: 2026-03-11T22:04:20.529Z
Learning: In `apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts`, the `msg.u._id === uid` early-return in the `streamNewMessage` handler is intentional: the "New messages" indicator is designed to notify about messages from other users only. Self-sent messages — including those sent from a different session/device — are always skipped, by design. Do not flag this as a multi-session regression.
Applied to files:
apps/meteor/server/slashcommands/msg/server.ts
📚 Learning: 2025-11-04T16:49:19.107Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.
Applied to files:
apps/meteor/server/slashcommands/leave/leave.tsapps/meteor/server/slashcommands/archiveroom/server.tsapps/meteor/server/lib/bot-helpers/index.ts
📚 Learning: 2026-03-09T18:39:21.178Z
Learnt from: Harxhit
Repo: RocketChat/Rocket.Chat PR: 39476
File: apps/meteor/server/methods/addAllUserToRoom.ts:0-0
Timestamp: 2026-03-09T18:39:21.178Z
Learning: In apps/meteor/server/methods/addAllUserToRoom.ts, the implementation uses a single cursor pass (Users.find(userFilter).batchSize(100)) that collects both the full user objects (collectedUsers: IUser[]) and their usernames (usernames: string[]) in one iteration. `beforeAddUserToRoom` is then called once with the full usernames batch (preserving batch-validation semantics), and the subsequent subscription/message processing loop iterates over the same stable `collectedUsers` array — no second DB query is made. This avoids any race condition between validation and processing while preserving the original batch-validation behavior.
Applied to files:
apps/meteor/server/slashcommands/leave/leave.tsapps/meteor/server/slashcommands/archiveroom/server.tsapps/meteor/server/lib/bot-helpers/index.ts
📚 Learning: 2025-12-09T20:01:07.355Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37532
File: ee/packages/federation-matrix/src/FederationMatrix.ts:920-927
Timestamp: 2025-12-09T20:01:07.355Z
Learning: In Rocket.Chat's federation invite handling (ee/packages/federation-matrix/src/FederationMatrix.ts), when a user rejects an invite via federationSDK.rejectInvite(), the subscription cleanup happens automatically through an event-driven flow: Matrix emits a leave event back, which is processed by handleLeave() in ee/packages/federation-matrix/src/events/member.ts, and that function calls Room.performUserRemoval() to clean up the subscription. No explicit cleanup is needed in the reject branch of handleInvite() because the leave event handler takes care of it.
<!-- </add_learning>
Applied to files:
apps/meteor/server/slashcommands/leave/leave.ts
📚 Learning: 2026-04-18T12:32:53.425Z
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 38623
File: apps/meteor/app/lib/server/functions/cleanRoomHistory.ts:146-149
Timestamp: 2026-04-18T12:32:53.425Z
Learning: In `apps/meteor/app/lib/server/functions/cleanRoomHistory.ts` (PR `#38623`), the read-receipt cleanup (both `ReadReceipts.removeByMessageIds` and `ReadReceiptsArchive.removeByMessageIds`) is intentionally only performed in the limited prune path (`limit && selectedMessageIds`). The unlimited/delete-all path (`limit === 0`) deliberately skips cleaning up orphaned read receipts in both hot and cold storage — this is by design. Do not flag this as a bug or missing cleanup in future reviews.
Applied to files:
apps/meteor/server/slashcommands/archiveroom/server.tsapps/meteor/MIGRATION_PLAN.md
📚 Learning: 2025-11-27T17:56:26.050Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37557
File: apps/meteor/client/views/admin/ABAC/AdminABACRooms.tsx:115-116
Timestamp: 2025-11-27T17:56:26.050Z
Learning: In Rocket.Chat, the GET /v1/abac/rooms endpoint (implemented in ee/packages/abac/src/index.ts) only returns rooms where abacAttributes exists and is not an empty array (query: { abacAttributes: { $exists: true, $ne: [] } }). Therefore, in components consuming this endpoint (like AdminABACRooms.tsx), room.abacAttributes is guaranteed to be defined for all returned rooms, and optional chaining before calling array methods like .join() is sufficient without additional null coalescing.
Applied to files:
apps/meteor/server/slashcommands/archiveroom/server.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/server/importPackages.tsapps/meteor/MIGRATION_PLAN.md
📚 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/server/importPackages.tsapps/meteor/scripts/migration/move-module.mjs
📚 Learning: 2025-09-19T15:15:04.642Z
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 36991
File: apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts:219-221
Timestamp: 2025-09-19T15:15:04.642Z
Learning: The Federation_Matrix_homeserver_domain setting in apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts is part of the old federation system and is being deprecated/removed, so configuration issues with this setting should not be flagged for improvement.
Applied to files:
apps/meteor/server/lib/bot-helpers/index.ts
📚 Learning: 2026-03-16T22:56:54.500Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 39677
File: packages/models/src/helpers/omnichannel/agentStatus.ts:10-29
Timestamp: 2026-03-16T22:56:54.500Z
Learning: In `packages/models/src/helpers/omnichannel/agentStatus.ts` (PR `#39677`), the `queryStatusAgentOnline` function intentionally omits the `$or` offline-status guard for non-bot agents when `isLivechatEnabledWhenAgentIdle === true`. This is by design: the setting `Livechat_enabled_when_agent_idle` (`accept_chats_when_agent_idle`) means agents should receive chats even when idle/offline, so the offline filter must be removed in that path. Bots are always status-agnostic and are always included regardless of their online/offline status. Do not flag this as a bug.
Applied to files:
apps/meteor/server/lib/bot-helpers/index.tsapps/meteor/MIGRATION_PLAN.md
📚 Learning: 2026-03-11T18:15:53.272Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39425
File: apps/meteor/app/api/server/v1/rooms.ts:294-296
Timestamp: 2026-03-11T18:15:53.272Z
Learning: In Rocket.Chat's `rooms.mediaConfirm/:rid/:fileId` endpoint (apps/meteor/app/api/server/v1/rooms.ts), updating `file.name` from `bodyParams.fileName` without updating `file.path` is intentionally safe. The file path is keyed by `_id` (not by filename), so the stored path remains valid regardless of a rename. `file.name` only affects the display name in the message attachment; do not flag this as a path-divergence issue.
Applied to files:
apps/meteor/scripts/migration/move-module.mjs
📚 Learning: 2026-04-20T17:11:59.452Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 40225
File: apps/meteor/ee/server/apps/communication/endpoints/appLogsHandler.ts:55-71
Timestamp: 2026-04-20T17:11:59.452Z
Learning: In `apps/meteor/ee/server/apps/communication/endpoints/appLogsHandler.ts`, the concern about an empty `?appId=` query param bypassing the truthy check and overriding the path `appId` in the `makeAppLogsQuery` spread is not relevant. The AJV query schema (`isAppLogsProps`) validates and rejects invalid/empty `appId` values before the action handler is reached, making the in-handler guard sufficient as-is. Do not flag this pattern as a vulnerability in future reviews of this file.
Applied to files:
apps/meteor/scripts/migration/move-module.mjs
📚 Learning: 2026-03-16T11:57:17.987Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 39657
File: apps/meteor/tests/end-to-end/apps/app-resolve-visitor.ts:43-45
Timestamp: 2026-03-16T11:57:17.987Z
Learning: In `apps/meteor/tests/end-to-end/apps/app-resolve-visitor.ts`, `externalIds` lookups are namespaced per `app.id`. Because `cleanupApps()` is called before each test run and a fresh app is installed (giving a new unique `app.id`), fixed `userId` strings like `'nonexistent-id'` in null-path tests are safe and cannot collide with stale visitor records from previous runs. Do not flag these as needing unique/random values.
Applied to files:
apps/meteor/scripts/migration/move-module.mjs
📚 Learning: 2026-03-18T16:47:56.781Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 39701
File: packages/apps-engine/src/server/runtime/deno/AppsEngineDenoRuntime.ts:137-151
Timestamp: 2026-03-18T16:47:56.781Z
Learning: In `packages/apps-engine/src/server/runtime/deno/AppsEngineDenoRuntime.ts`, the `EEXIST` catch block in the `DenoRuntimeSubprocessController` constructor (around lines 144–151) intentionally skips validating the target of a pre-existing `deno-runtime` symlink in the temp directory. The rationale is that spoofing the symlink requires the attacker to already have system-level write access to the temp directory, which grants far greater control than the deno-runtime sandbox could provide. Do not flag this as a security issue in future reviews.
Applied to files:
apps/meteor/scripts/migration/move-module.mjs
📚 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/MIGRATION_PLAN.md
📚 Learning: 2026-03-16T21:50:42.118Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39676
File: .changeset/migrate-users-register-openapi.md:3-3
Timestamp: 2026-03-16T21:50:42.118Z
Learning: In RocketChat/Rocket.Chat OpenAPI migration PRs, removing endpoint types and validators from `rocket.chat/rest-typings` (e.g., `UserRegisterParamsPOST`, `/v1/users.register` entry) is the *required* migration pattern per RocketChat/Rocket.Chat-Open-API#150 Rule 7 ("No More rest-typings or Manual Typings"). The endpoint type is re-exposed via a module augmentation `.d.ts` file in the consuming package (e.g., `packages/web-ui-registration/src/users-register.d.ts`). This is NOT a breaking change — the correct changeset bump for `rocket.chat/rest-typings` in this scenario is `minor`, not `major`. Do not flag this as a breaking change during OpenAPI migration reviews.
Applied to files:
apps/meteor/MIGRATION_PLAN.md
📚 Learning: 2026-03-20T13:52:29.575Z
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 39553
File: apps/meteor/app/api/server/v1/stats.ts:98-117
Timestamp: 2026-03-20T13:52:29.575Z
Learning: In `apps/meteor/app/api/server/v1/stats.ts`, the `statistics.telemetry` POST endpoint intentionally has no `body` AJV schema in its route options. The proper request body shape (a `params` array of telemetry event objects) has not been formally defined yet, so body validation is deferred to a follow-up. Do not flag the missing body schema for this endpoint during OpenAPI migration reviews.
Applied to files:
apps/meteor/MIGRATION_PLAN.md
📚 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/MIGRATION_PLAN.md
📚 Learning: 2026-03-06T18:09:17.867Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsx:20-23
Timestamp: 2026-03-06T18:09:17.867Z
Learning: In the RocketChat/Rocket.Chat gazzodown package (`packages/gazzodown`), tests are intended to run under the UTC timezone, but as of PR `#39397` this is NOT yet explicitly enforced in `jest.config.ts` or the `package.json` test scripts (which just run `jest` without `TZ=UTC`). To make timezone-sensitive snapshot tests reliable across all environments, `TZ=UTC` should be added to the test scripts in `package.json` or to `jest.config.ts` via `testEnvironmentOptions.timezone`. Without explicit UTC enforcement, snapshot tests involving date-fns formatted output or `toLocaleString()` will fail for contributors in non-UTC timezones.
Applied to files:
apps/meteor/MIGRATION_PLAN.md
📚 Learning: 2026-02-23T17:53:18.785Z
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.
Applied to files:
apps/meteor/MIGRATION_PLAN.md
📚 Learning: 2026-03-14T14:58:58.834Z
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.
Applied to files:
apps/meteor/MIGRATION_PLAN.md
🪛 LanguageTool
apps/meteor/MIGRATION_PLAN.md
[uncategorized] ~431-~431: The official name of this software platform is spelled with a capital “H”.
Context: ...uth-providers/drupal.ts | |app/github/server/ |server...
(GITHUB)
[uncategorized] ~431-~431: The official name of this software platform is spelled with a capital “H”.
Context: .../github/server/ |server/lib/auth-providers/github.ts | |app/github-enterpri...
(GITHUB)
[uncategorized] ~436-~436: The official name of this content management system is spelled with a capital “P”.
Context: ...uth-providers/iframe.ts | |app/wordpress/server/ |server/li...
(WORDPRESS)
[uncategorized] ~436-~436: The official name of this content management system is spelled with a capital “P”.
Context: .../wordpress/server/ |server/lib/auth-providers/wordpress.ts | |app/lib/server/oauth/*...
(WORDPRESS)
[style] ~611-~611: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ds//` but continue to work. - No new abstractions. No port interfaces,...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~612-~612: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ection, no new TypeScript patterns. - **No reorganization of existing server/ fi...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 markdownlint-cli2 (0.22.1)
apps/meteor/MIGRATION_PLAN.md
[warning] 11-11: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 23-23: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (8)
apps/meteor/MIGRATION_PLAN.md (2)
11-13: Fenced code blocks still missing language tags (MD040).Already flagged previously; noting here as not yet addressed so it stays on the radar.
Also applies to: 23-23
203-203: Phase 2 header folder count vs. table mismatch.Previously flagged; Phase 2 header still says "5 folders" while the table lists 7 source rows.
apps/meteor/server/slashcommands/create/server.ts (1)
5-9: LGTM — import paths align with the new module location.Relative paths correctly resolve from
apps/meteor/server/slashcommands/create/back toapps/meteor/app/...and toapps/meteor/server/lib/i18n. No behavioral change.apps/meteor/server/slashcommands/msg/server.ts (1)
6-10: LGTM.Import rewrites are consistent with the new path;
../../methods/createDirectMessagecorrectly resolves toapps/meteor/server/methods/createDirectMessage.apps/meteor/server/slashcommands/leave/leave.ts (1)
6-9: LGTM.Relative paths correctly re-anchored to the new location.
apps/meteor/server/lib/bot-helpers/index.ts (1)
8-13: LGTM — import paths correctly re-anchored forserver/lib/bot-helpers/.
../../../app/...reaches the app tree and../../methods/removeUserFromRoomresolves toapps/meteor/server/methods/removeUserFromRoom. Behavior preserved.apps/meteor/server/importPackages.ts (1)
8-8: LGTM — import wiring matches the new slashcommand and bot-helpers layout.Each
./slashcommands/<name>import resolves to the correspondingserver/slashcommands/<name>/index.ts, and./lib/bot-helperspicks up the newserver/lib/bot-helpers/index.ts. Since slash commands are leaf modules, relocating these imports in the list is safe.Also applies to: 48-64
apps/meteor/server/slashcommands/archiveroom/server.ts (1)
7-13: LGTM.All relative imports correctly re-anchored to the new location; no logic changes.
d-gubert
reviewed
Apr 23, 2026
Updates Phase 5 destinations and the existing server/methods/ folder to server/meteor-methods/ to make it explicit these are Meteor RPC handlers (deprecated), not generic class/REST/service methods. Adds a Phase 5 first-step instruction to git mv the existing folder before moving new files in. Also switches verification commands across all phases from `tsc --noEmit` to `yarn lint --quiet` as the authoritative check for broken imports after file moves. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
KevLehman
approved these changes
May 4, 2026
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
3 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.
Summary
scripts/migration/) for moving modules and updating imports automaticallyapp/*/server/intoserver/commands/with corrected relative importsserver/importPackages.tsto reference the new locationsThis is Phase 1 of a broader backend reorganization that consolidates
app/*/server/modules intoserver/following a responsibility-first folder structure. Slash commands are leaf nodes (no inbound dependencies), making them the safest starting point.Moved modules
asciiarts,archiveroom,ban,create,help,hide,invite,inviteall,join,kick,leave,me,msg,mute,status,topic,unarchiveroom,bot-helpersMigration scripts (disposable)
move-module.mjs— moves a directory viagit mv, updates all relative imports (internal + external)move-batch.mjs— reads a TSV manifest and runs move-module for each entryverify-no-old-imports.mjs— scans for stale imports referencing old pathsThese scripts will be removed after all migration phases are complete.
Test plan
app/slashcommand*orapp/bot-helpers/serverpathstsc --noEmit— no new errors introduced (pre-existing errors from missing node_modules are unrelated)/gimme,/ban,/kick, etc.)git log --follow)Summary by CodeRabbit
Documentation
Refactor
Chores