chore: remove backend auto-close unclosed markdown code blocks (dead code after 9.0.0)#40356
Open
billywithaw1lly wants to merge 3 commits intoRocketChat:release-9.0.0from
Open
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 |
🦋 Changeset detectedLatest commit: 5384ab6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 41 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
WalkthroughRemoves server-side auto-closing of unclosed markdown code blocks, which is now handled client-side as of version 9.0.0. Includes changelog entries and removal of the corresponding regression test. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.changeset/integration-scripts-no-babel.md (1)
1-14:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix changeset mismatch: “Breaking” text but
@rocket.chat/meteoris set topatch.This changeset describes a breaking change (“Breaking: …”), but the version bump is classified as
patch. That can lead to incorrect release versioning/upgrade expectations.Suggested alignment options
--- -'@rocket.chat/meteor': patch +'@rocket.chat/meteor': major ---OR, if it’s truly not breaking anymore, update the body to remove/soften the “Breaking” label (so the text matches the
patchbump).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.changeset/integration-scripts-no-babel.md around lines 1 - 14, The changeset for '@rocket.chat/meteor' is inconsistent: the header sets the release type to "patch" but the body contains a "Breaking:" label; update the changeset so the header and body match by either (A) changing the release type from 'patch' to the appropriate breaking level (major or minor) in the frontmatter for '@rocket.chat/meteor', or (B) if this is not actually breaking, remove or soften the "Breaking:" label and wording in the body so it correctly reflects a patch; ensure the '@rocket.chat/meteor' identifier and the "Breaking:" text are consistently updated.
🧹 Nitpick comments (2)
.changeset/thin-news-rhyme.md (1)
5-5: ⚡ Quick winCapitalize “Markdown” (proper noun) for consistency.
Line 5 uses lowercase “markdown”. Consider changing to “Markdown” for consistent terminology and to avoid language-tool nits.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.changeset/thin-news-rhyme.md at line 5, Update the phrasing in the changeset text to capitalize the proper noun "Markdown" (replace the lowercase "markdown" with "Markdown") so the sentence reads "Remove backend auto-close unclosed Markdown code blocks (handled client-side since 9.0.0)"; locate the phrase in the .changeset/thin-news-rhyme.md content and change only the word casing to maintain consistency..changeset/integration-scripts-no-babel.md (1)
5-5: ⚡ Quick winCapitalize “Markdown” (proper noun) for consistency.
Line 5 says “... unclosed markdown code blocks ...” (lowercase). Consider changing to “Markdown” to match the rest of the ecosystem and avoid style/language-tool warnings.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.changeset/integration-scripts-no-babel.md at line 5, The text uses lowercase "markdown" in the sentence containing "unclosed markdown code blocks"; update that occurrence to "Markdown" in the .changeset/integration-scripts-no-babel.md diff so the phrase reads "unclosed Markdown code blocks" to match proper noun capitalization and maintain consistency across the docs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In @.changeset/integration-scripts-no-babel.md:
- Around line 1-14: The changeset for '@rocket.chat/meteor' is inconsistent: the
header sets the release type to "patch" but the body contains a "Breaking:"
label; update the changeset so the header and body match by either (A) changing
the release type from 'patch' to the appropriate breaking level (major or minor)
in the frontmatter for '@rocket.chat/meteor', or (B) if this is not actually
breaking, remove or soften the "Breaking:" label and wording in the body so it
correctly reflects a patch; ensure the '@rocket.chat/meteor' identifier and the
"Breaking:" text are consistently updated.
---
Nitpick comments:
In @.changeset/integration-scripts-no-babel.md:
- Line 5: The text uses lowercase "markdown" in the sentence containing
"unclosed markdown code blocks"; update that occurrence to "Markdown" in the
.changeset/integration-scripts-no-babel.md diff so the phrase reads "unclosed
Markdown code blocks" to match proper noun capitalization and maintain
consistency across the docs.
In @.changeset/thin-news-rhyme.md:
- Line 5: Update the phrasing in the changeset text to capitalize the proper
noun "Markdown" (replace the lowercase "markdown" with "Markdown") so the
sentence reads "Remove backend auto-close unclosed Markdown code blocks (handled
client-side since 9.0.0)"; locate the phrase in the
.changeset/thin-news-rhyme.md content and change only the word casing to
maintain consistency.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0ffdac58-ea26-4a1a-a291-f476b364f44e
📒 Files selected for processing (4)
.changeset/integration-scripts-no-babel.md.changeset/thin-news-rhyme.mdapps/meteor/server/services/messages/service.tsapps/meteor/tests/end-to-end/api/chat.ts
💤 Files with no reviewable changes (2)
- apps/meteor/server/services/messages/service.ts
- apps/meteor/tests/end-to-end/api/chat.ts
📜 Review details
🧰 Additional context used
🧠 Learnings (12)
📓 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: 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.
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 0
File: :0-0
Timestamp: 2026-02-24T19:05:56.710Z
Learning: In Rocket.Chat PRs, keep feature PRs free of unrelated lockfile-only dependency bumps; prefer reverting lockfile drift or isolating such bumps into a separate "chore" commit/PR, and always use yarn install --immutable with the Yarn version pinned in package.json via Corepack.
Learnt from: 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.
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 0
File: :0-0
Timestamp: 2026-02-24T19:05:56.710Z
Learning: Rocket.Chat repo context: When a workspace manifest on develop already pins a dependency version (e.g., packages/web-ui-registration → "rocket.chat/ui-contexts": "27.0.1"), a lockfile change in a feature PR that upgrades only that dependency’s resolution is considered a manifest-driven sync and can be kept, preferably as a small "chore: sync yarn.lock with manifests" commit.
📚 Learning: 2026-03-16T21:50:37.589Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39676
File: .changeset/migrate-users-register-openapi.md:3-3
Timestamp: 2026-03-16T21:50:37.589Z
Learning: For changes related to OpenAPI migrations in Rocket.Chat/OpenAPI, when removing endpoint types and validators from rocket.chat/rest-typings (e.g., UserRegisterParamsPOST, /v1/users.register) document this as a minor changeset (not breaking) per RocketChat/Rocket.Chat-Open-API#150 Rule 7. Note that the endpoint type is re-exposed via a module augmentation .d.ts in the consuming package (e.g., packages/web-ui-registration/src/users-register.d.ts). In reviews, ensure the changeset clearly states: this is a non-breaking change, the major version should not be bumped, and the changeset reflects a minor version bump. Do not treat this as a breaking change during OpenAPI migrations.
Applied to files:
.changeset/integration-scripts-no-babel.md.changeset/thin-news-rhyme.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:
.changeset/integration-scripts-no-babel.md.changeset/thin-news-rhyme.md
📚 Learning: 2026-02-24T19:05:56.710Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 0
File: :0-0
Timestamp: 2026-02-24T19:05:56.710Z
Learning: Rocket.Chat repo context: When a workspace manifest on develop already pins a dependency version (e.g., packages/web-ui-registration → "rocket.chat/ui-contexts": "27.0.1"), a lockfile change in a feature PR that upgrades only that dependency’s resolution is considered a manifest-driven sync and can be kept, preferably as a small "chore: sync yarn.lock with manifests" commit.
Applied to files:
.changeset/integration-scripts-no-babel.md.changeset/thin-news-rhyme.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:
.changeset/integration-scripts-no-babel.md.changeset/thin-news-rhyme.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:
.changeset/integration-scripts-no-babel.md.changeset/thin-news-rhyme.md
📚 Learning: 2026-02-24T19:05:56.710Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 0
File: :0-0
Timestamp: 2026-02-24T19:05:56.710Z
Learning: In Rocket.Chat PRs, keep feature PRs free of unrelated lockfile-only dependency bumps; prefer reverting lockfile drift or isolating such bumps into a separate "chore" commit/PR, and always use yarn install --immutable with the Yarn version pinned in package.json via Corepack.
Applied to files:
.changeset/integration-scripts-no-babel.md.changeset/thin-news-rhyme.md
📚 Learning: 2026-04-14T21:10:36.233Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 36292
File: apps/meteor/client/hooks/useHasValidLocationHash.ts:7-12
Timestamp: 2026-04-14T21:10:36.233Z
Learning: In the RocketChat/Rocket.Chat repository, adding JSDoc-style comments to React hooks (e.g., files under apps/meteor/client/hooks/) is considered a good pattern and should not be flagged as a violation of the "avoid code comments in the implementation" guideline. The guideline against code comments does not apply to JSDoc documentation on exported hooks.
Applied to files:
.changeset/integration-scripts-no-babel.md.changeset/thin-news-rhyme.md
📚 Learning: 2026-03-17T16:08:37.572Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 39690
File: packages/ui-voip/package.json:11-11
Timestamp: 2026-03-17T16:08:37.572Z
Learning: In `packages/ui-voip/package.json` (RocketChat/Rocket.Chat), the team deliberately chose to use `rm -rf dist` directly in the `"build"` script instead of `rimraf`, as they decided against introducing the `rimraf` dependency for this package. Do not flag `rm -rf dist` in the ui-voip build script as a cross-platform issue requiring rimraf.
Applied to files:
.changeset/integration-scripts-no-babel.md
📚 Learning: 2025-11-19T18:20:07.720Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37419
File: packages/i18n/src/locales/en.i18n.json:918-921
Timestamp: 2025-11-19T18:20:07.720Z
Learning: Repo: RocketChat/Rocket.Chat — i18n/formatting
Learning: This repository uses a custom message formatting parser in UI blocks/messages; do not assume standard Markdown rules. For keys like Call_ended_bold, Call_not_answered_bold, Call_failed_bold, and Call_transferred_bold in packages/i18n/src/locales/en.i18n.json, retain the existing single-asterisk emphasis unless maintainers request otherwise.
Applied to files:
.changeset/integration-scripts-no-babel.md.changeset/thin-news-rhyme.md
📚 Learning: 2026-03-23T19:33:46.159Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 39818
File: apps/meteor/client/hooks/usePruneWarningMessage.ts:45-56
Timestamp: 2026-03-23T19:33:46.159Z
Learning: In RocketChat/Rocket.Chat, cron expressions used by the retention policy (e.g., `RetentionPolicy_Advanced_Precision_Cron`) follow the convention of the `cron` npm package (outdated version), which uses **0-indexed months** (0 = January, 11 = December), not the standard 1-12 used in POSIX cron. The local `sendAt` function in `apps/meteor/client/hooks/usePruneWarningMessage.ts` intentionally replicates this 0-indexed behavior. Do NOT suggest adding `+1` to `date.getMonth()` in this file, as it would break tests and create inconsistency with the backend.
Applied to files:
.changeset/integration-scripts-no-babel.md
📚 Learning: 2025-11-10T19:06:20.146Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37408
File: apps/meteor/client/views/admin/ABAC/useRoomAttributeOptions.tsx:53-69
Timestamp: 2025-11-10T19:06:20.146Z
Learning: In the Rocket.Chat repository, do not provide suggestions or recommendations about code sections marked with TODO comments. The maintainers have already identified these as future work and external reviewers lack the full context about implementation plans and timing.
Applied to files:
.changeset/thin-news-rhyme.md
🪛 LanguageTool
.changeset/thin-news-rhyme.md
[uncategorized] ~5-~5: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ...--- Remove backend auto-close unclosed markdown code blocks (handled client-side since ...
(MARKDOWN_NNP)
🔇 Additional comments (1)
.changeset/integration-scripts-no-babel.md (1)
1-6: 💤 Low valueRequest verification: ensure this changeset belongs in this PR.
This file’s content appears unrelated to “remove backend auto-close unclosed markdown code blocks”; it’s about webhook integration scripts and Babel/isolated-vm. Please confirm it’s intentionally included in this PR (or the intended target) so we don’t publish misleading release notes.
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
2 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.
Remove the backend logic that auto-closes unclosed markdown code blocks, along with its corresponding test. In 9.0.0, this behavior is handled entirely on the client side, making the server-side implementation dead code. Both the code and test contain TODO comments explicitly marking them for removal in 9.0.0.
closeUnclosedCodeBlockcall and imports fromapps/meteor/server/services/messages/service.tsshouldBreakInVersionimport fromapps/meteor/server/services/messages/service.tsapps/meteor/tests/end-to-end/api/chat.tsCloses #39033
No functional changes for 9.0.0+ — client side already handles this. Verify no remaining references:
grep -rn "closeUnclosedCodeBlock|shouldBreakInVersion" apps/meteor/server/services/messages/service.ts
Straightforward cleanup targeting the release-9.0.0 branch. No behavior changes expected.
Summary by CodeRabbit
Refactor
Tests
Chores