Conversation
|
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 |
WalkthroughThe Changesajv Dependency Bump
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #41049 +/- ##
===========================================
- Coverage 70.16% 70.09% -0.08%
===========================================
Files 3361 3361
Lines 129690 129690
Branches 22490 22482 -8
===========================================
- Hits 91000 90909 -91
- Misses 35378 35463 +85
- Partials 3312 3318 +6
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@package.json`:
- Around line 61-66: The resolutions block is missing an exact pin for the new
ajv descriptor, so requests for ajv@npm:^8.20.0 will not be covered. Update the
package.json resolutions entry near the existing ajv pins to add ajv@npm:^8.20.0
mapped to the same 8.20.0 version, keeping it alongside the other ajv resolution
keys.
🪄 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: adc39909-4266-4ca7-88b6-611b91ac1b1e
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (7)
apps/meteor/ee/server/services/package.jsonapps/meteor/package.jsonpackage.jsonpackages/http-router/package.jsonpackages/livechat/package.jsonpackages/media-signaling/package.jsonpackages/rest-typings/package.json
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: Hacktron Security Check
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2026-06-16T14:13:34.463Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 40974
File: packages/web-ui-registration/package.json:31-31
Timestamp: 2026-06-16T14:13:34.463Z
Learning: In Rocket.Chat’s monorepo, when reviewing a dependency entry and flagging that a specific version “does not exist” (e.g., in package.json), first verify the exact package/version directly against the npm registry (use URLs like https://registry.npmjs.org/<package>/<version> or https://www.npmjs.com/package/<package>/v/<version>). Do not rely on web search results for this check, since they may be stale or cached and may not reflect the latest published versions.
Applied to files:
packages/http-router/package.jsonpackages/livechat/package.jsonpackages/rest-typings/package.jsonpackages/media-signaling/package.json
📚 Learning: 2026-06-16T14:13:49.795Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 40974
File: packages/web-ui-registration/package.json:26-26
Timestamp: 2026-06-16T14:13:49.795Z
Learning: During code reviews that check whether a dependency version exists in package.json (especially for Rocket.Chat’s rocket.chat/fuselage and related rocket.chat/fuselage-* packages), don’t rely on web search results. Instead, verify the version directly against the npm registry (e.g., via the npm registry API or the canonical package URL https://www.npmjs.com/package/<package>/v/<version>) before deciding that a version bump is invalid. If the version is present in the npm registry, do not flag it as invalid.
Applied to files:
packages/http-router/package.jsonpackages/livechat/package.jsonpackages/rest-typings/package.jsonpackages/media-signaling/package.json
📚 Learning: 2026-06-16T14:13:59.986Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 40974
File: packages/ui-video-conf/package.json:25-25
Timestamp: 2026-06-16T14:13:59.986Z
Learning: In the Rocket.Chat monorepo, when reviewing a dependency version bump for rocket.chat/fuselage in a package.json, do not flag the new version constraint as “non-existent” or invalid unless you verify the published versions directly from the npm registry (https://www.npmjs.com/package/rocket.chat/fuselage). Don’t rely on search/web results for available versions since they can be stale.
Applied to files:
packages/http-router/package.jsonpackages/livechat/package.jsonpackages/rest-typings/package.jsonpackages/media-signaling/package.json
🔇 Additional comments (6)
apps/meteor/package.json (1)
169-169: LGTM!apps/meteor/ee/server/services/package.json (1)
30-30: LGTM!packages/http-router/package.json (1)
23-23: LGTM!packages/livechat/package.json (1)
36-36: LGTM!packages/media-signaling/package.json (1)
33-33: LGTM!packages/rest-typings/package.json (1)
20-20: LGTM!
| "ajv@npm:^6.12.4": "6.15.0", | ||
| "ajv@npm:^6.12.5": "6.15.0", | ||
| "ajv@npm:^6.14.0": "6.15.0", | ||
| "ajv@npm:^8.0.0": "8.20.0", | ||
| "ajv@npm:^8.0.1": "8.20.0", | ||
| "ajv@npm:^8.9.0": "8.20.0", |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Ajv resolution entries in root package.json:"
python - <<'PY'
import json
from pathlib import Path
pkg = json.loads(Path("package.json").read_text())
for k,v in pkg.get("resolutions", {}).items():
if k.startswith("ajv@npm:"):
print(f"{k} -> {v}")
PY
echo
echo "Ajv descriptors currently present in yarn.lock:"
rg -n '^"ajv@npm:\^' yarn.lock || true
echo
echo "Workspace package manifests requesting ajv ranges:"
fd -p package.json | xargs rg -n '"ajv"\s*:\s*"\^'Repository: RocketChat/Rocket.Chat
Length of output: 804
🏁 Script executed:
sed -n '1,140p' package.jsonRepository: RocketChat/Rocket.Chat
Length of output: 5896
🏁 Script executed:
sed -n '1,140p' package.jsonRepository: RocketChat/Rocket.Chat
Length of output: 5896
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "package.json relevant section:"
sed -n '1,140p' package.json
echo
echo "yarn.lock ajv-related entries:"
rg -n '(^"|ajv@npm:|ajv@)' yarn.lock | head -n 80Repository: RocketChat/Rocket.Chat
Length of output: 10217
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "package.json relevant section:"
sed -n '1,140p' package.json
echo
echo "yarn.lock ajv-related entries:"
rg -n '(^\"|ajv@npm:|ajv@)' yarn.lock | head -n 80Repository: RocketChat/Rocket.Chat
Length of output: 6047
Add ajv@npm:^8.20.0 to the resolutions block.
The current pins cover ^8.0.0, ^8.0.1, and ^8.9.0, but workspaces now depend on ajv: "^8.20.0". Without an exact resolution for that descriptor, the pin won’t apply to those requests.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@package.json` around lines 61 - 66, The resolutions block is missing an exact
pin for the new ajv descriptor, so requests for ajv@npm:^8.20.0 will not be
covered. Update the package.json resolutions entry near the existing ajv pins to
add ajv@npm:^8.20.0 mapped to the same 8.20.0 version, keeping it alongside the
other ajv resolution keys.
There was a problem hiding this comment.
1 issue found across 8 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="package.json">
<violation number="1" location="package.json:66">
P2: For consistency and to prevent future version drift, add `"ajv@npm:^8.20.0": "8.20.0"` to the resolutions block. The existing pins only cover `^8.0.0`, `^8.0.1`, and `^8.9.0` (for transitive dependencies), but the workspace packages now directly depend on `^8.20.0`. Yarn resolutions must match the exact range descriptor — without a matching entry, a future `yarn install` could resolve `^8.20.0` to a newer version (e.g., 8.21.0) bypassing the intended pin.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| "ajv@npm:^6.14.0": "6.15.0", | ||
| "ajv@npm:^8.0.0": "8.20.0", | ||
| "ajv@npm:^8.0.1": "8.20.0", | ||
| "ajv@npm:^8.9.0": "8.20.0", |
There was a problem hiding this comment.
P2: For consistency and to prevent future version drift, add "ajv@npm:^8.20.0": "8.20.0" to the resolutions block. The existing pins only cover ^8.0.0, ^8.0.1, and ^8.9.0 (for transitive dependencies), but the workspace packages now directly depend on ^8.20.0. Yarn resolutions must match the exact range descriptor — without a matching entry, a future yarn install could resolve ^8.20.0 to a newer version (e.g., 8.21.0) bypassing the intended pin.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At package.json, line 66:
<comment>For consistency and to prevent future version drift, add `"ajv@npm:^8.20.0": "8.20.0"` to the resolutions block. The existing pins only cover `^8.0.0`, `^8.0.1`, and `^8.9.0` (for transitive dependencies), but the workspace packages now directly depend on `^8.20.0`. Yarn resolutions must match the exact range descriptor — without a matching entry, a future `yarn install` could resolve `^8.20.0` to a newer version (e.g., 8.21.0) bypassing the intended pin.</comment>
<file context>
@@ -58,6 +58,12 @@
+ "ajv@npm:^6.14.0": "6.15.0",
+ "ajv@npm:^8.0.0": "8.20.0",
+ "ajv@npm:^8.0.1": "8.20.0",
+ "ajv@npm:^8.9.0": "8.20.0",
"fast-xml-parser@npm:5.3.6": "^5.5.7",
"fast-xml-parser/fast-xml-builder": "^1.1.9",
</file context>
| "ajv@npm:^8.9.0": "8.20.0", | |
| "ajv@npm:^8.9.0": "8.20.0", | |
| "ajv@npm:^8.20.0": "8.20.0", |
Proposed changes (including videos or screenshots)
Issue(s)
https://rocketchat.atlassian.net/browse/SB-1000
Steps to test or reproduce
Further comments
Summary by CodeRabbit