Skip to content

chore: Add OpenAPI Support to e2e.fetchMyKeys API#36779

Merged
ggazzo merged 13 commits intoRocketChat:developfrom
ahmed-n-abdeltwab:feat/openapi-e2e-fetchMyKeys
Feb 27, 2026
Merged

chore: Add OpenAPI Support to e2e.fetchMyKeys API#36779
ggazzo merged 13 commits intoRocketChat:developfrom
ahmed-n-abdeltwab:feat/openapi-e2e-fetchMyKeys

Conversation

@ahmed-n-abdeltwab
Copy link
Copy Markdown
Contributor

@ahmed-n-abdeltwab ahmed-n-abdeltwab commented Aug 23, 2025

Description:
This PR integrates OpenAPI support into the Rocket.Chat API, migrate of Rocket.Chat API endpoints to the new OpenAPI pattern. The update includes improved API documentation, enhanced type safety, and response validation using AJV.

Key Changes:

  • Implemented the new pattern and added AJV-based JSON schema validation for API.
  • Uses the ExtractRoutesFromAPI utility from the TypeScript definitions to dynamically derive the routes from the endpoint specifications.
  • Enabled Swagger UI integration for this API.
  • Route Methods Chaining for the endpoints.
  • This does not introduce any breaking changes to the endpoint logic.

Issue Reference:
Relates to #34983, part of the ongoing OpenAPI integration effort.

Testing:

  • Verified that the API response schemas are correctly documented in Swagger UI.
  • All tests passed without any breaking changes
$ 
    

Endpoints:

Get E2E Keys

Looking forward to your feedback! 🚀

Summary by CodeRabbit

  • New Features

    • API documentation added for end-to-end key retrieval.
  • Improvements

    • Stronger authentication enforced for e2e key access.
    • Strict, validated responses for key retrieval to improve reliability and type safety.
    • Internal API surface streamlined for clearer endpoint behavior.

Task: ARCH-2011

[COMM-144]

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Aug 23, 2025

🦋 Changeset detected

Latest commit: 3f8e112

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 41 packages
Name Type
@rocket.chat/meteor Minor
@rocket.chat/rest-typings Minor
@rocket.chat/api-client Patch
@rocket.chat/core-services Patch
@rocket.chat/ddp-client Patch
@rocket.chat/http-router Patch
@rocket.chat/models Patch
@rocket.chat/ui-contexts Major
@rocket.chat/web-ui-registration Major
@rocket.chat/account-service Patch
@rocket.chat/authorization-service Patch
@rocket.chat/ddp-streamer Patch
@rocket.chat/federation-matrix Patch
@rocket.chat/omnichannel-services Patch
@rocket.chat/presence Patch
rocketchat-services Patch
@rocket.chat/omnichannel-transcript Patch
@rocket.chat/presence-service Patch
@rocket.chat/queue-worker Patch
@rocket.chat/abac Patch
@rocket.chat/network-broker Patch
@rocket.chat/omni-core-ee Patch
@rocket.chat/livechat Patch
@rocket.chat/mock-providers Patch
@rocket.chat/cron Patch
@rocket.chat/instance-status Patch
@rocket.chat/omni-core Patch
@rocket.chat/server-fetch Patch
@rocket.chat/ui-client Major
@rocket.chat/media-calls Patch
@rocket.chat/uikit-playground Patch
@rocket.chat/fuselage-ui-kit Major
@rocket.chat/gazzodown Major
@rocket.chat/ui-avatar Major
@rocket.chat/ui-video-conf Major
@rocket.chat/ui-voip Major
@rocket.chat/core-typings Minor
@rocket.chat/apps Patch
@rocket.chat/model-typings Patch
@rocket.chat/license Patch
@rocket.chat/pdf-worker Patch

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

@dionisio-bot
Copy link
Copy Markdown
Contributor

dionisio-bot bot commented Aug 23, 2025

Looks like this PR is ready to merge! 🎉
If you have any trouble, please check the PR guidelines

@codecov
Copy link
Copy Markdown

codecov bot commented Aug 23, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.71%. Comparing base (8d73ce5) to head (3f8e112).
⚠️ Report is 75 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop   #36779      +/-   ##
===========================================
+ Coverage    70.65%   70.71%   +0.05%     
===========================================
  Files         3190     3191       +1     
  Lines       112732   112968     +236     
  Branches     20431    20497      +66     
===========================================
+ Hits         79649    79882     +233     
- Misses       31035    31039       +4     
+ Partials      2048     2047       -1     
Flag Coverage Δ
e2e 60.37% <ø> (+0.03%) ⬆️
e2e-api 48.81% <ø> (+0.94%) ⬆️
unit 71.32% <ø> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ahmed-n-abdeltwab ahmed-n-abdeltwab marked this pull request as ready for review August 23, 2025 17:03
@ahmed-n-abdeltwab ahmed-n-abdeltwab requested review from a team as code owners August 23, 2025 17:03
@ahmed-n-abdeltwab
Copy link
Copy Markdown
Contributor Author

Hey @cardoso, feel free to review whenever you have time

@ahmed-n-abdeltwab
Copy link
Copy Markdown
Contributor Author

👍 @cardoso

@ahmed-n-abdeltwab ahmed-n-abdeltwab marked this pull request as draft February 23, 2026 18:38
@ggazzo ggazzo added this to the 8.3.0 milestone Feb 23, 2026
@ahmed-n-abdeltwab ahmed-n-abdeltwab changed the title feat: Add OpenAPI Support to e2e.fetchMyKeys API chore: Add OpenAPI Support to e2e.fetchMyKeys API Feb 24, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Feb 24, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6101980 and 3f8e112.

📒 Files selected for processing (1)
  • apps/meteor/app/api/server/v1/e2e.ts
📜 Recent review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: 📦 Build Packages
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}

📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)

**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation

Files:

  • apps/meteor/app/api/server/v1/e2e.ts
🧠 Learnings (11)
📓 Common learnings
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38974
File: apps/meteor/app/api/server/v1/im.ts:220-221
Timestamp: 2026-02-24T19:09:09.561Z
Learning: In RocketChat/Rocket.Chat OpenAPI migration PRs for apps/meteor/app/api/server/v1 endpoints, maintainers prefer to avoid any logic changes; style-only cleanups (like removing inline comments) may be deferred to follow-ups to keep scope tight.
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 0
File: :0-0
Timestamp: 2026-02-24T19:05:56.710Z
Learning: In Rocket.Chat PRs, keep feature PRs free of unrelated lockfile-only dependency bumps; prefer reverting lockfile drift or isolating such bumps into a separate "chore" commit/PR, and always use yarn install --immutable with the Yarn version pinned in package.json via Corepack.
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 0
File: :0-0
Timestamp: 2026-02-24T19:05:56.710Z
Learning: Rocket.Chat repo context: When a workspace manifest on develop already pins a dependency version (e.g., packages/web-ui-registration → "rocket.chat/ui-contexts": "27.0.1"), a lockfile change in a feature PR that upgrades only that dependency’s resolution is considered a manifest-driven sync and can be kept, preferably as a small "chore: sync yarn.lock with manifests" commit.
Learnt from: 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: 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.
📚 Learning: 2026-02-24T19:09:01.522Z
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:01.522Z
Learning: In Rocket.Chat OpenAPI migration PRs for endpoints under apps/meteor/app/api/server/v1, avoid introducing logic changes. Only perform scope-tight changes that preserve behavior; style-only cleanups (e.g., removing inline comments) may be deferred to follow-ups to keep the migration PR focused.

Applied to files:

  • apps/meteor/app/api/server/v1/e2e.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/app/api/server/v1/e2e.ts
📚 Learning: 2026-01-17T01:51:47.764Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38219
File: packages/core-typings/src/cloud/Announcement.ts:5-6
Timestamp: 2026-01-17T01:51:47.764Z
Learning: In packages/core-typings/src/cloud/Announcement.ts, the AnnouncementSchema.createdBy field intentionally overrides IBannerSchema.createdBy (object with _id and optional username) with a string enum ['cloud', 'system'] to match existing runtime behavior. This is documented as technical debt with a FIXME comment at apps/meteor/app/cloud/server/functions/syncWorkspace/handleCommsSync.ts:53 and should not be flagged as an error until the runtime behavior is corrected.

Applied to files:

  • apps/meteor/app/api/server/v1/e2e.ts
📚 Learning: 2026-01-26T18:26:01.279Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 38227
File: apps/meteor/app/api/server/router.ts:44-49
Timestamp: 2026-01-26T18:26:01.279Z
Learning: In apps/meteor/app/api/server/router.ts, when retrieving bodyParams and queryParams from the Hono context via c.get(), do not add defensive defaults (e.g., ?? {}). The code should fail fast if these parameters are missing, as endpoint handlers expect them to be present and breaking here helps surface parsing problems rather than hiding them.

Applied to files:

  • apps/meteor/app/api/server/v1/e2e.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 : Use `expect` matchers for assertions (`toEqual`, `toContain`, `toBeTruthy`, `toHaveLength`, etc.) instead of `assert` statements in Playwright tests

Applied to files:

  • apps/meteor/app/api/server/v1/e2e.ts
📚 Learning: 2026-02-12T15:39:28.416Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 32703
File: apps/meteor/client/lib/chats/flows/uploadFiles.ts:52-58
Timestamp: 2026-02-12T15:39:28.416Z
Learning: In `apps/meteor/client/lib/chats/flows/uploadFiles.ts`, when E2E encryption is required but not allowed (e.g., `E2E_Enable_Encrypt_Files` setting is disabled), the function intentionally abandons the entire upload queue and displays a toast error. This fail-fast behavior prevents partial uploads when encryption requirements cannot be met and is the expected behavior, not a bug.

Applied to files:

  • apps/meteor/app/api/server/v1/e2e.ts
📚 Learning: 2026-02-23T17:53:06.802Z
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:06.802Z
Learning: During PR reviews that touch endpoint files under apps/meteor/app/api/server/v1, enforce strict scope: if a PR targets a specific endpoint (e.g., rooms.favorite), do not propose changes to unrelated endpoints (e.g., rooms.invite) unless maintainers explicitly request them. Focus feedback on the touched endpoint's behavior, API surface, and related tests; avoid broad cross-endpoint changes in the same PR unless requested.

Applied to files:

  • apps/meteor/app/api/server/v1/e2e.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/app/api/server/v1/e2e.ts
📚 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:

  • apps/meteor/app/api/server/v1/e2e.ts
📚 Learning: 2026-02-24T19:05:56.710Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 0
File: :0-0
Timestamp: 2026-02-24T19:05:56.710Z
Learning: Rocket.Chat repo context: When a workspace manifest on develop already pins a dependency version (e.g., packages/web-ui-registration → "rocket.chat/ui-contexts": "27.0.1"), a lockfile change in a feature PR that upgrades only that dependency’s resolution is considered a manifest-driven sync and can be kept, preferably as a small "chore: sync yarn.lock with manifests" commit.

Applied to files:

  • apps/meteor/app/api/server/v1/e2e.ts
🧬 Code graph analysis (1)
apps/meteor/app/api/server/v1/e2e.ts (2)
packages/models/src/index.ts (1)
  • Users (192-192)
apps/meteor/app/api/server/index.ts (1)
  • API (51-51)
🔇 Additional comments (1)
apps/meteor/app/api/server/v1/e2e.ts (1)

99-123: LGTM! Endpoint migration preserves legacy behavior correctly.

The e2e.fetchMyKeys endpoint is properly migrated to the chained route pattern with:

  • authRequired: true for authenticated access
  • Response schema allowing optional public_key/private_key fields (matching the model's { public_key, private_key } | object return type)
  • No failure branch introduced, preserving the legacy contract where clients handle absent keys

This aligns with the OpenAPI migration scope without introducing logic changes. Based on learnings: "In Rocket.Chat OpenAPI migration PRs for endpoints under apps/meteor/app/api/server/v1, avoid introducing logic changes."


Walkthrough

Migrates e2e.fetchMyKeys from an ad-hoc API.v1 route to a chained route with OpenAPI metadata and AJV validation, adding an auth requirement and a strict 200 response schema returning public_key, private_key, and success.

Changes

Cohort / File(s) Summary
Endpoint Migration
apps/meteor/app/api/server/v1/e2e.ts
Replaced API.v1.addRoute ad-hoc handler with a chained e2e.fetchMyKeys route under e2eEndpoints, set authRequired: true, added OpenAPI response schemas and validation, and updated handler to return { public_key, private_key, success } via Users.fetchKeysByUserId.
Type Definitions
packages/rest-typings/src/v1/e2e.ts
Removed the old /v1/e2e.fetchMyKeys GET entry from E2eEndpoints type declarations to align with the new chained route definitions.
Changeset
.changeset/short-starfishes-provide.md
Added note about OpenAPI support for e2e.fetchMyKeys, AJV schema usage, and bumped minor versions for @rocket.chat/meteor and @rocket.chat/rest-typings.

Sequence Diagram(s)

sequenceDiagram
  participant Client as Client
  participant API as API Server
  participant Users as Users Model/DB

  Client->>API: GET /v1/e2e.fetchMyKeys (auth token)
  API->>API: authenticate request
  API->>Users: Users.fetchKeysByUserId(userId)
  Users-->>API: { public_key, private_key } | or null
  API-->>Client: 200 { public_key, private_key, success:true } / 401/400
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'chore: Add OpenAPI Support to e2e.fetchMyKeys API' accurately and clearly describes the main change: migrating the e2e.fetchMyKeys endpoint to use OpenAPI support with proper schema validation.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@ahmed-n-abdeltwab
Copy link
Copy Markdown
Contributor Author

ahmed-n-abdeltwab commented Feb 24, 2026

@coderabbitai would it be okay if I bumped @rocket.chat/ui-contexts to 27.0.1? It seems to be updating to this version automatically. Would that be considered unrelated to this PR, and should I revert it to 27.0.0? Also, what are the best practices for this project when dealing with Yarn change?

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Feb 24, 2026

Tip

For best results, initiate chat on the files or code changes.

The PR description image could not be loaded in my environment (likely a private GitHub user-attachment). Proceeding based on the PR contents I can access.

@ahmed-n-abdeltwab Great question. Short answer: since this PR is about OpenAPI for e2e.fetchMyKeys, the @rocket.chat/ui-contexts 27.0.1 bump looks unrelated and came in via lockfile drift. I’d recommend reverting it in this PR to keep scope tight, per prior feedback/preferences on Rocket.Chat reviews. I’m basing this on an earlier learning I have for this repo about keeping PRs narrowly scoped.

What I’m seeing

  • This PR now has a commit “chore: bump @rocket.chat/ui-contexts to 27.0.1” dated Feb 24, 2026. (github.com)
  • The only code change tied to that bump is in yarn.lock, where ui-contexts went from 27.0.0 to 27.0.1 in a peer metadata block. (github.com)
  • The repo pins Yarn via packageManager to [email protected] (and engines.yarn 4.12.0). Avoid changing this in feature PRs. (raw.githubusercontent.com)

Recommendation

  • Revert in this PR: keep it focused on the API migration.
    • How: reset the lockfile to develop, then install immutably to avoid re-generating it.
      • git checkout origin/develop -- yarn.lock
      • corepack use 4.12.0
      • yarn install --immutable
    • Drop the bump commit if it contains only the lockfile change.
  • If you prefer to keep 27.0.1:
    • Keep it as a separate “chore:” commit (which you already have), or move it to a quick, separate PR so it doesn’t block/entangle this one.

Best practices for Yarn changes in this repo

  • Respect the pinned Yarn version: the repo uses packageManager: "[email protected]". Don’t run yarn set version or modify .yarn releases in feature PRs. (raw.githubusercontent.com)
  • Use immutable installs locally to avoid accidental lockfile churn:
    • corepack enable && corepack use 4.12.0
    • yarn install --immutable
  • Avoid blanket dependency bumps (yarn up / dedupe) in feature PRs. If a bump is required, isolate it in a dedicated chore PR/commit.
  • If a lockfile change appears after merging develop (due to workspace/semver resolution), prefer either:
    • Revert it to keep the feature PR clean; or
    • Keep it as a separate “chore: lockfile refresh” commit if maintainers are okay with it.

If you’d like, I can outline exact git steps for either path (revert here vs. separate PR) based on how you want to proceed. (github.com)


🧠 Learnings used
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:06.802Z
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: 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.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Feb 24, 2026

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

@coderabbit-user
I couldn’t view any PR description images in this thread. If there’s a screenshot to consider, please re-attach it, and I’ll incorporate it.

Short answer

  • Yes, the @rocket.chat/ui-contexts 27.0.1 bump is unrelated to this OpenAPI-focused PR.
  • Prefer reverting the lockfile drift here and keep the PR scope tight. This aligns with prior maintainer guidance we have as a learning for this repo about avoiding cross-scope changes.

What likely happened

  • One or more workspace manifests allow a semver range like ^27.0.0 for @rocket.chat/ui-contexts, so a fresh install or a non-immutable install updated the lockfile to 27.0.1.

What I found in this branch (from the script above)

  • Pinned Yarn version and any engines in package.json.
  • Whether .yarnrc.yml exists and key settings.
  • Where @rocket.chat/ui-contexts is declared in workspace package.json files and what ranges they use.
  • Whether yarn.lock contains @rocket.chat/ui-contexts and if it changed vs origin/develop.

Recommendation
Option A — Revert the drift in this PR (preferred)

  1. Reset the lockfile to develop and reinstall immutably:
    • git checkout origin/develop -- yarn.lock
    • corepack enable && corepack use $(jq -r '.packageManager' package.json | cut -d@ -f2)
    • yarn install --immutable
  2. Commit as chore: revert lockfile drift (ui-contexts back to 27.0.0).
  3. Push to keep this PR focused on OpenAPI.

Option B — Keep 27.0.1 but isolate it

  • Either keep it as a separate “chore:” commit in this PR with a clear message, or move it to a tiny follow-up PR:
    • yarn up -R @rocket.chat/ui-contexts@27.0.1
    • Verify Storybook/build if relevant, then commit as chore: bump @rocket.chat/ui-contexts to 27.0.1.

Best practices for Yarn changes in this repo

  • Always use the pinned Yarn version from package.json (packageManager) via Corepack:
    • corepack enable
    • corepack use yarn@
  • Avoid changing Yarn itself (don’t run yarn set version or modify .yarn releases) in feature PRs.
  • Use immutable installs to prevent accidental lockfile churn:
    • yarn install --immutable
  • Keep dependency bumps out of feature PRs. If necessary, isolate them in a dedicated “chore:” PR/commit.
  • If you must refresh the lockfile due to conflicts when merging develop, do it in a dedicated chore commit with a concise message like chore: refresh yarn.lock after merge conflicts, and avoid sneaking in unrelated package updates.

If you’d like, tell me which option you prefer (revert here vs. quick follow-up chore PR), and I’ll outline exact git commands accordingly.


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Git branch and remotes =="
git branch --show-current || true
git remote -v || true

echo
echo "== Pinned Yarn and engines from package.json =="
if command -v jq >/dev/null 2>&1; then
  jq -r '"packageManager=\(.packageManager // "N/A")\nengines.yarn=\(.engines.yarn // "N/A")\nengines.node=\(.engines.node // "N/A")"' package.json 2>/dev/null || true
else
  rg -n '"packageManager"|"yarn"|"node"' package.json -n -C0 || true
fi

echo
echo "== .yarnrc.yml (first 60 lines if present) =="
if [ -f .yarnrc.yml ]; then
  sed -n '1,60p' .yarnrc.yml
else
  echo "(no .yarnrc.yml)"
fi

echo
echo "== Workspace manifests that depend on `@rocket.chat/ui-contexts` =="
rg -nP --type=json -C1 '"@rocket\.chat/ui-contexts"\s*:\s*"[^\"]+"' || true

echo
echo "== yarn.lock entries mentioning `@rocket.chat/ui-contexts` (context) =="
if [ -f yarn.lock ]; then
  rg -n -C2 '@rocket\.chat/ui-contexts' yarn.lock || true
else
  echo "(no yarn.lock)"
fi

echo
echo "== Compare yarn.lock against develop (if available) =="
git fetch origin develop --depth=1 >/dev/null 2>&1 || true
if git rev-parse --verify origin/develop >/dev/null 2>&1; then
  if git diff --name-only origin/develop...HEAD | rg -q '^yarn\.lock$'; then
    echo "yarn.lock differs from origin/develop"
    # Show only small diff hunk around ui-contexts to avoid noise
    git diff -U0 origin/develop -- yarn.lock | rg -n -C2 '@rocket\.chat/ui-contexts' || echo "(ui-contexts not found in yarn.lock diff)"
  else
    echo "yarn.lock does not differ from origin/develop"
  fi
else
  echo "(could not verify diff vs origin/develop)"
fi

Length of output: 10160


@ahmed-n-abdeltwab thanks for running that. Here’s what your script output shows:

What we now know

  • Yarn is pinned to 4.12.0 and Node to 22.16.0; the repo uses node-modules linker.
  • Many packages depend on @rocket.chat/ui-contexts via workspace ranges (workspace:^ / ~).
  • packages/web-ui-registration explicitly declares @rocket.chat/ui-contexts in two places:
    • as workspace:^ (likely devDependencies), and
    • as a pinned external version 27.0.1 (likely dependencies).
  • The only yarn.lock delta vs origin/develop is indeed 27.0.0 → 27.0.1 for @rocket.chat/ui-contexts.

Implications

  • The lockfile change is consistent with a manifest that pins 27.0.1. Reverting only yarn.lock to 27.0.0 would fight the manifest and will likely fail yarn install --immutable.
  • Whether this is “unrelated scope” depends on if this PR changed the manifest (packages/web-ui-registration/package.json). If that manifest was already 27.0.1 on develop, then your one-line lock update is just bringing the lockfile in sync, which is reasonable. If the manifest changed here, then it’s unrelated to the OpenAPI work and should be isolated.

Recommendation

  • If web-ui-registration already had 27.0.1 on develop: keep the lockfile update (or put it in a small chore commit).
  • If this PR changed that manifest: either
    • move that change to a separate “chore: bump @rocket.chat/ui-contexts to 27.0.1 in web-ui-registration” commit/PR, or
    • revert the manifest to 27.0.0 and reset the lockfile (not recommended unless there’s a reason to hold back 27.0.1).

Best practices for Yarn changes in this repo (reiterating maintainer guidance about tight scope, per our previous learning)

  • Use the pinned Yarn via Corepack: corepack enable && corepack use [email protected].
  • Prefer yarn install --immutable locally to avoid accidental lockfile churn (even though enableImmutableInstalls is false in .yarnrc.yml).
  • Don’t change Yarn itself (.yarn/releases, yarnPath) in feature PRs.
  • Keep dependency bumps out of feature PRs; if needed, isolate them into a small “chore:” commit/PR.
  • If a manifest pins a version (like 27.0.1), don’t try to force the lockfile back to an older version; make the decision at the manifest level.

Would you like me to confirm whether packages/web-ui-registration/package.json was modified by this PR? If yes, I’ll run the quick diff below.

I’m following the prior maintainer preference we learned (keep PRs narrowly scoped). Depending on the diff result above, I can outline exact git steps for either keeping this as a tiny chore commit or reverting it cleanly.


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Did packages/web-ui-registration/package.json change vs origin/develop? =="
git fetch origin develop --depth=1 >/dev/null 2>&1 || true
if git rev-parse --verify origin/develop >/dev/null 2>&1; then
  if git diff --name-only origin/develop...HEAD | rg -q '^packages/web-ui-registration/package.json$'; then
    echo "Changed."
    echo
    echo "-- Diff of only the `@rocket.chat/ui-contexts` lines --"
    git diff -U0 origin/develop...HEAD -- packages/web-ui-registration/package.json | rg -n -C1 '"@rocket\.chat/ui-contexts"\s*:\s*"[^\"]+"' || true
    echo
    echo "-- Full minimal diff for context --"
    git diff -U3 origin/develop...HEAD -- packages/web-ui-registration/package.json || true
  else
    echo "No changes detected in packages/web-ui-registration/package.json."
    echo
    echo "-- Current `@rocket.chat/ui-contexts` lines for reference --"
    rg -nP -C1 '"@rocket\.chat/ui-contexts"\s*:\s*"[^\"]+"' packages/web-ui-registration/package.json || true
  fi
else
  echo "(could not verify against origin/develop)"
fi

Length of output: 458


The PR description image could not be loaded in my environment, and there’s no Comment image provided in this thread.

@ahmed-n-abdeltawab thanks for the script output—super helpful.

What your results show

  • Tooling pins: Yarn 4.12.0 (packageManager and engines) and Node 22.16.0.
  • .yarnrc.yml uses nodeLinker: node-modules and enableImmutableInstalls: false.
  • packages/web-ui-registration/package.json already pins @rocket.chat/ui-contexts to 27.0.1 (Line 64) and also references it via workspace:^ elsewhere (Line 36). This file did not change in this PR compared to origin/develop.
  • The only yarn.lock delta vs origin/develop is 27.0.0 → 27.0.1 for @rocket.chat/ui-contexts.

Conclusion

  • Keep 27.0.1. The lockfile update is manifest-driven and not introduced by this PR; reverting yarn.lock to 27.0.0 would conflict with the existing manifest and likely fail yarn install --immutable.
  • To keep scope tidy per maintainer preference (learning we follow here), you can keep this as a tiny “chore” change or separate commit.

Suggested next step

  • If you want a clear paper trail:
    • git add yarn.lock
    • git commit -m "chore: sync yarn.lock with manifests (@rocket.chat/ui-contexts 27.0.1)"
    • git push

Yarn best practices for this repo

  • Use the pinned Yarn via Corepack:
  • Prefer immutable installs locally to avoid accidental lockfile churn (even though enableImmutableInstalls is false):
    • yarn install --immutable
  • Don’t change Yarn itself (.yarn/releases, yarnPath) in feature PRs.
  • Keep dependency bumps out of feature PRs; if needed, isolate them into a small “chore:” commit/PR with a clear message.
  • If a manifest pins a version (as with web-ui-registration → 27.0.1), let the lockfile follow the manifest rather than forcing an older resolution.

If you’d like, I can also scan for any other non-workspace-pinned references to @rocket.chat/ui-contexts that might cause similar lockfile diffs in future and report back.


✏️ Learnings added
Learnt from: ahmed-n-abdeltwab
URL: https://github.com/RocketChat/Rocket.Chat/pull/36779

Timestamp: 2026-02-24T19:05:56.689Z
Learning: In Rocket.Chat PRs, keep feature PRs free of unrelated lockfile-only dependency bumps; prefer reverting lockfile drift or isolating such bumps into a separate "chore" commit/PR, and always use yarn install --immutable with the Yarn version pinned in package.json via Corepack.

Learnt from: ahmed-n-abdeltwab
URL: https://github.com/RocketChat/Rocket.Chat/pull/36779

Timestamp: 2026-02-24T19:05:56.689Z
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.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: 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.

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:06.802Z
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: 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.

Add OpenAPI support for e2e.fetchMyKeys endpoints by migrating to a modern route definition syntax and utilizing shared AJV schemas for validation.
@ahmed-n-abdeltwab ahmed-n-abdeltwab marked this pull request as ready for review February 25, 2026 18:34
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 3 files

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/meteor/app/api/server/v1/e2e.ts`:
- Around line 107-115: The 200 response schema compiled via ajv.compile<{
public_key: string; private_key: string }> currently only requires 'success',
allowing responses without keys; update the schema used in the ajv.compile call
so the required array includes 'public_key', 'private_key', and 'success' (i.e.,
required: ['public_key', 'private_key', 'success']) to enforce both key fields
are present in successful responses.
- Around line 121-123: The new check that calls API.v1.failure when 'public_key'
or 'private_key' are missing changes prior behavior; revert to returning the
model result directly instead of failing: remove the if-block that references
result and API.v1.failure and ensure the handler returns the original result
value (the variable result) as before; if you intend to change
contract/behavior, move this change into a separate behavior-change PR with
tests.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8d73ce5 and 6101980.

📒 Files selected for processing (3)
  • .changeset/short-starfishes-provide.md
  • apps/meteor/app/api/server/v1/e2e.ts
  • packages/rest-typings/src/v1/e2e.ts
💤 Files with no reviewable changes (1)
  • packages/rest-typings/src/v1/e2e.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). (1)
  • GitHub Check: CodeQL-Build
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}

📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)

**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation

Files:

  • apps/meteor/app/api/server/v1/e2e.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38974
File: apps/meteor/app/api/server/v1/im.ts:220-221
Timestamp: 2026-02-24T19:09:09.561Z
Learning: In RocketChat/Rocket.Chat OpenAPI migration PRs for apps/meteor/app/api/server/v1 endpoints, maintainers prefer to avoid any logic changes; style-only cleanups (like removing inline comments) may be deferred to follow-ups to keep scope tight.
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 0
File: :0-0
Timestamp: 2026-02-24T19:05:56.710Z
Learning: Rocket.Chat repo context: When a workspace manifest on develop already pins a dependency version (e.g., packages/web-ui-registration → "rocket.chat/ui-contexts": "27.0.1"), a lockfile change in a feature PR that upgrades only that dependency’s resolution is considered a manifest-driven sync and can be kept, preferably as a small "chore: sync yarn.lock with manifests" commit.
Learnt from: 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: 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: 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.
📚 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/short-starfishes-provide.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/short-starfishes-provide.md
📚 Learning: 2026-02-24T19:09:01.522Z
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:01.522Z
Learning: In Rocket.Chat OpenAPI migration PRs for endpoints under apps/meteor/app/api/server/v1, avoid introducing logic changes. Only perform scope-tight changes that preserve behavior; style-only cleanups (e.g., removing inline comments) may be deferred to follow-ups to keep the migration PR focused.

Applied to files:

  • apps/meteor/app/api/server/v1/e2e.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/app/api/server/v1/e2e.ts
📚 Learning: 2026-02-23T17:53:06.802Z
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:06.802Z
Learning: During PR reviews that touch endpoint files under apps/meteor/app/api/server/v1, enforce strict scope: if a PR targets a specific endpoint (e.g., rooms.favorite), do not propose changes to unrelated endpoints (e.g., rooms.invite) unless maintainers explicitly request them. Focus feedback on the touched endpoint's behavior, API surface, and related tests; avoid broad cross-endpoint changes in the same PR unless requested.

Applied to files:

  • apps/meteor/app/api/server/v1/e2e.ts
🧬 Code graph analysis (1)
apps/meteor/app/api/server/v1/e2e.ts (1)
apps/meteor/app/api/server/index.ts (1)
  • API (51-51)
🔇 Additional comments (1)
.changeset/short-starfishes-provide.md (1)

1-6: Changeset entry looks scoped and consistent with the migration.

Minor bumps and summary text match the endpoint OpenAPI migration work in this PR.

@ahmed-n-abdeltwab
Copy link
Copy Markdown
Contributor Author

ahmed-n-abdeltwab commented Feb 26, 2026

@ggazzo 👍

@ggazzo ggazzo added the stat: QA assured Means it has been tested and approved by a company insider label Feb 27, 2026
@dionisio-bot dionisio-bot bot added the stat: ready to merge PR tested and approved waiting for merge label Feb 27, 2026
@dionisio-bot dionisio-bot bot enabled auto-merge February 27, 2026 14:36
@ggazzo ggazzo disabled auto-merge February 27, 2026 15:17
@ggazzo ggazzo merged commit d8baf39 into RocketChat:develop Feb 27, 2026
48 checks passed
@ggazzo
Copy link
Copy Markdown
Member

ggazzo commented Feb 27, 2026

/jira ARCH-1935

@ahmed-n-abdeltwab
Copy link
Copy Markdown
Contributor Author

thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community stat: QA assured Means it has been tested and approved by a company insider stat: ready to merge PR tested and approved waiting for merge type: chore

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants