fix: migrate federation add user tests from DDP to REST responses#40740
Open
yummyPancake2607 wants to merge 1 commit into
Open
fix: migrate federation add user tests from DDP to REST responses#40740yummyPancake2607 wants to merge 1 commit into
yummyPancake2607 wants to merge 1 commit 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
WalkthroughThis PR migrates test helpers and E2E test assertions from DDP method calls to REST API endpoints for user room invitations. The ChangesREST API Migration for User Room Invitations
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
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. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
ee/packages/federation-matrix/tests/end-to-end/permissions.spec.ts (1)
525-541:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winUndefined variable causes test to fail at runtime.
The test references
addUserResponseon lines 538-540, but this variable is never assigned. ThecreateRoomcall on line 528 discards its return value.🐛 Proposed fix
it('should NOT be able to be added to a federated room during creation', async () => { const channelName = `federated-room-${Date.now()}`; - await createRoom({ + const createResponse = await createRoom({ type: 'p', name: channelName, members: [userWithNonMatchingEmail.username], extraData: { federated: true, }, config: userRequestConfig, }); - expect(addUserResponse.status).toBe(400); - expect(addUserResponse.body).toHaveProperty('success', false); - expect(addUserResponse.body).toHaveProperty('error', 'error-not-authorized-federation'); + expect(createResponse.status).toBe(400); + expect(createResponse.body).toHaveProperty('success', false); + expect(createResponse.body).toHaveProperty('error', 'error-not-authorized-federation'); });🤖 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 `@ee/packages/federation-matrix/tests/end-to-end/permissions.spec.ts` around lines 525 - 541, The test references an undefined addUserResponse because createRoom(...)'s return value was discarded; update the test to capture the response from createRoom by assigning its result to addUserResponse (or a newly declared variable) and then assert on that variable; locate the createRoom call in the test and ensure the returned response object from createRoom is awaited and stored (e.g., const addUserResponse = await createRoom(...)) before the expect(...) assertions that check status/body.apps/meteor/tests/data/rooms.helper.ts (1)
119-131:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStale doc comment: still describes the removed DDP path.
This JSDoc was not updated with the migration — it references the
addUsersToRoom"DDP method call" and a "method call response," and omits the newtypeparameter, which now misdescribes the function.📝 Suggested doc update
-/** - * Adds users to a room using the addUsersToRoom method. - * - * Invites one or more users to join a room using the DDP method call. - * Supports both local and federated users, with proper error handling - * for federation restrictions. - * - * `@param` usernames - Array of usernames to add to the room - * `@param` rid - The unique identifier of the room - * `@param` userCredentials - Optional credentials for the request (deprecated, use config instead) - * `@param` config - Optional request configuration for custom domains - * `@returns` Promise resolving to the method call response - */ +/** + * Adds users to a room using the REST channels.invite / groups.invite endpoints. + * + * Invites one or more users to join a room by issuing one REST request per + * username in parallel. Supports both local and federated users, with proper + * error handling for federation restrictions. + * + * `@param` usernames - Array of usernames to add to the room + * `@param` rid - The unique identifier of the room + * `@param` type - Room type discriminator: 'c' (channel) or 'p' (private group) + * `@param` userCredentials - Optional credentials for the request (deprecated, use config instead) + * `@param` config - Optional request configuration for custom domains + * `@returns` Promise resolving to an array of REST invite responses (one per username) + */🤖 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 `@apps/meteor/tests/data/rooms.helper.ts` around lines 119 - 131, Update the stale JSDoc for the addUsersToRoom helper to remove references to a "DDP method call" and "method call response", and instead describe the current implementation and return value; explicitly document the new type parameter (e.g., invite type) along with existing parameters userCredentials and config, and state the actual Promise return (e.g., Promise<void> or the resolved payload) and behavior for local vs federated users and federation restrictions; ensure parameter names match the function signature (addUsersToRoom, type, usernames, rid, userCredentials, config) and keep the comment concise and accurate.
🤖 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 `@apps/meteor/tests/data/rooms.helper.ts`:
- Around line 202-211: The REST helper in rooms.helper.ts posts to
api('commands.run') which returns API.v1.success({ result }) (no DDP `message`
envelope), so the federation test in
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts must stop parsing
response.body.message; update the test to assert against response.body.result
(or switch to a DDP-based helper), specifically replace
JSON.parse(response.body.message) and checks for msg:'result' with direct
assertions on response.body.result to match the /v1/commands.run implementation.
- Around line 132-166: The addUserToRoom helper now breaks callers by requiring
type and always returning an array; make the helper backward-compatible by
making the type parameter optional (default to 'c' when missing), update the
return shape to return the single request response when usernames.length === 1
and an array only when multiple usernames are provided, and update the JSDoc for
addUserToRoom to document the optional type and return shape; also fix the
affected federation-matrix tests (e.g., permissions.spec.ts, dms.spec.ts,
room.spec.ts) that parse commands.run responses to read response.body.result
(not response.body.message) so they match the server route that returns
API.v1.success({ result }).
---
Outside diff comments:
In `@apps/meteor/tests/data/rooms.helper.ts`:
- Around line 119-131: Update the stale JSDoc for the addUsersToRoom helper to
remove references to a "DDP method call" and "method call response", and instead
describe the current implementation and return value; explicitly document the
new type parameter (e.g., invite type) along with existing parameters
userCredentials and config, and state the actual Promise return (e.g.,
Promise<void> or the resolved payload) and behavior for local vs federated users
and federation restrictions; ensure parameter names match the function signature
(addUsersToRoom, type, usernames, rid, userCredentials, config) and keep the
comment concise and accurate.
In `@ee/packages/federation-matrix/tests/end-to-end/permissions.spec.ts`:
- Around line 525-541: The test references an undefined addUserResponse because
createRoom(...)'s return value was discarded; update the test to capture the
response from createRoom by assigning its result to addUserResponse (or a newly
declared variable) and then assert on that variable; locate the createRoom call
in the test and ensure the returned response object from createRoom is awaited
and stored (e.g., const addUserResponse = await createRoom(...)) before the
expect(...) assertions that check status/body.
🪄 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: e1a16805-42d7-4fe4-a397-1cbde6bb5489
📒 Files selected for processing (4)
apps/meteor/tests/data/rooms.helper.tsee/packages/federation-matrix/tests/end-to-end/dms.spec.tsee/packages/federation-matrix/tests/end-to-end/permissions.spec.tsee/packages/federation-matrix/tests/end-to-end/room.spec.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: cubic · AI code reviewer
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{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:
ee/packages/federation-matrix/tests/end-to-end/room.spec.tsee/packages/federation-matrix/tests/end-to-end/dms.spec.tsapps/meteor/tests/data/rooms.helper.tsee/packages/federation-matrix/tests/end-to-end/permissions.spec.ts
**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.spec.ts: Use descriptive test names that clearly communicate expected behavior in Playwright tests
Use.spec.tsextension for test files (e.g.,login.spec.ts)
Files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.tsee/packages/federation-matrix/tests/end-to-end/dms.spec.tsee/packages/federation-matrix/tests/end-to-end/permissions.spec.ts
🧠 Learnings (6)
📚 Learning: 2025-12-09T20:01:00.324Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37532
File: ee/packages/federation-matrix/src/FederationMatrix.ts:920-927
Timestamp: 2025-12-09T20:01:00.324Z
Learning: When reviewing federation invite handling in Rocket.Chat (specifically under ee/packages/federation-matrix), understand that rejecting an invite via federationSDK.rejectInvite() triggers an event-driven cleanup: a leave event is emitted and handled by handleLeave() in ee/packages/federation-matrix/src/events/member.ts, which calls Room.performUserRemoval() to remove the subscription. Do not add explicit cleanup in the reject branch of handleInvite(); rely on the existing leave-event flow for cleanup. If making changes, ensure this invariant remains and that any related paths still funnel cleanup through the leave event to avoid duplicate or missing removals.
Applied to files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.tsee/packages/federation-matrix/tests/end-to-end/dms.spec.tsee/packages/federation-matrix/tests/end-to-end/permissions.spec.ts
📚 Learning: 2026-02-24T19:22:48.358Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/omnichannel/omnichannel-send-pdf-transcript.spec.ts:66-67
Timestamp: 2026-02-24T19:22:48.358Z
Learning: In Playwright end-to-end tests (e.g., under apps/meteor/tests/e2e/...), prefer locating elements by translated text (getByText) and ARIA roles (getByRole) over data-qa attributes. If translation values change, update the corresponding test locators accordingly. Never use data-qa locators. This guideline applies to all Playwright e2e test specs in the repository and helps keep tests robust to UI text changes and accessible semantics.
Applied to files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.tsee/packages/federation-matrix/tests/end-to-end/dms.spec.tsee/packages/federation-matrix/tests/end-to-end/permissions.spec.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:
ee/packages/federation-matrix/tests/end-to-end/room.spec.tsee/packages/federation-matrix/tests/end-to-end/dms.spec.tsapps/meteor/tests/data/rooms.helper.tsee/packages/federation-matrix/tests/end-to-end/permissions.spec.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:
ee/packages/federation-matrix/tests/end-to-end/room.spec.tsee/packages/federation-matrix/tests/end-to-end/dms.spec.tsapps/meteor/tests/data/rooms.helper.tsee/packages/federation-matrix/tests/end-to-end/permissions.spec.ts
📚 Learning: 2026-03-06T18:10:15.268Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/code/CodeBlock.spec.tsx:47-68
Timestamp: 2026-03-06T18:10:15.268Z
Learning: In tests (especially those using testing-library/dom/jsdom) for Rocket.Chat components, the HTML <code> element has an implicit ARIA role of 'code'. Therefore, screen.getByRole('code') or screen.findByRole('code') will locate <code> elements even without a role attribute. Do not flag findByRole('code') as invalid in reviews; prefer using the implicit role instead of adding role="code" unless necessary for accessibility.
Applied to files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.tsee/packages/federation-matrix/tests/end-to-end/dms.spec.tsee/packages/federation-matrix/tests/end-to-end/permissions.spec.ts
📚 Learning: 2026-05-06T12:21:44.083Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 40256
File: apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx:121-149
Timestamp: 2026-05-06T12:21:44.083Z
Learning: Field wrappers in rocket.chat/fuselage-forms (Field, FieldLabel, FieldRow, FieldError, FieldHint) auto-create htmlFor/id associations, aria-describedby, and role="alert" for errors. Do not manually set htmlFor, id, aria-describedby, or role attributes when using these wrappers. This automatic wiring does not apply to plain rocket.chat/fuselage components, which require explicit ID wiring per the accessibility docs. In code reviews, prefer using fuselage-forms wrappers for form fields and verify there is no unnecessary manual ID/aria wiring in files that use these wrappers. If a component uses plain fuselage components, ensure proper id wiring as per docs.
Applied to files:
ee/packages/federation-matrix/tests/end-to-end/room.spec.tsee/packages/federation-matrix/tests/end-to-end/dms.spec.tsapps/meteor/tests/data/rooms.helper.tsee/packages/federation-matrix/tests/end-to-end/permissions.spec.ts
🔇 Additional comments (9)
ee/packages/federation-matrix/tests/end-to-end/dms.spec.ts (3)
690-699: LGTM!
1176-1185: LGTM!
1743-1751: LGTM!ee/packages/federation-matrix/tests/end-to-end/permissions.spec.ts (4)
182-192: LGTM!
500-523: LGTM!
583-606: LGTM!
647-670: LGTM!ee/packages/federation-matrix/tests/end-to-end/room.spec.ts (1)
229-256: LGTM!apps/meteor/tests/data/rooms.helper.ts (1)
156-165:/v1/channels.inviteand/v1/groups.inviteacceptusernamein the request body
ChannelsInvitePropsallowsusername?: stringwith{ roomId: string }.GroupsInviteProps(viaWithUserIdinpackages/rest-typings/src/v1/groups/BaseProps.ts) includes variants whereusername: stringis accepted (as an alternative touserId/user) withadditionalProperties: false.
Comment on lines
132
to
166
| export const addUserToRoom = ({ | ||
| usernames, | ||
| rid, | ||
| type, | ||
| userCredentials, | ||
| config, | ||
| }: { | ||
| usernames: string[]; | ||
| rid: IRoom['_id']; | ||
| type: 'c' | 'p'; | ||
| userCredentials?: Credentials; | ||
| config?: IRequestConfig; | ||
| }) => { | ||
| if (!usernames || usernames.length === 0) { | ||
| throw new Error('"usernames" is required in "addUserToRoom" test helper'); | ||
| } | ||
|
|
||
| if (!rid) { | ||
| throw new Error('"rid" is required in "addUserToRoom" test helper'); | ||
| } | ||
|
|
||
| const requestInstance = config?.request || request; | ||
| const credentialsInstance = config?.credentials || userCredentials || credentials; | ||
|
|
||
| return requestInstance | ||
| .post(methodCall('addUsersToRoom')) | ||
| .set(credentialsInstance) | ||
| .send({ | ||
| message: JSON.stringify({ | ||
| method: 'addUsersToRoom', | ||
| params: [{ rid, users: usernames }], | ||
| id: 'id', | ||
| msg: 'method', | ||
| const endpoint = type === 'c' ? 'channels.invite' : 'groups.invite'; | ||
|
|
||
| return Promise.all( | ||
| usernames.map((username) => | ||
| requestInstance.post(api(endpoint)).set(credentialsInstance).send({ | ||
| roomId: rid, | ||
| username, | ||
| }), | ||
| }); | ||
| ), | ||
| ); | ||
| }; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all call sites of addUserToRoom to confirm they pass the new required `type`
# and handle the new array (Promise.all) return shape.
rg -nP --type=ts -C4 '\baddUserToRoom\s*\(' -g '!**/rooms.helper.ts'Repository: RocketChat/Rocket.Chat
Length of output: 35419
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Inspect rooms.helper.ts around the referenced regions
sed -n '90,240p' apps/meteor/tests/data/rooms.helper.tsRepository: RocketChat/Rocket.Chat
Length of output: 4425
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 2) Find how addUserToRoom is imported/used in federation-matrix tests
for f in \
ee/packages/federation-matrix/tests/end-to-end/permissions.spec.ts \
ee/packages/federation-matrix/tests/end-to-end/dms.spec.ts \
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts \
apps/meteor/tests/end-to-end/api/chat.ts
do
echo "===== $f (imports) ====="
rg -n "addUserToRoom" "$f" | head -n 20
echo
echo "===== $f (around first call) ====="
# print 25 lines around first occurrence
line=$(rg -n "addUserToRoom" "$f" | head -n 1 | cut -d: -f1)
start=$((line-15)); end=$((line+25))
sed -n "${start},${end}p" "$f"
echo
doneRepository: RocketChat/Rocket.Chat
Length of output: 2671
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 3) Inspect slash-command usage around the helper usage in room.spec.ts
file="ee/packages/federation-matrix/tests/end-to-end/room.spec.ts"
rg -n "addUserToRoomSlashCommand|commands\\.run|invite" "$file" | head -n 80
# show around occurrences of addUserToRoomSlashCommand
for line in $(rg -n "addUserToRoomSlashCommand" "$file" | cut -d: -f1); do
start=$((line-20)); end=$((line+30))
echo "===== $file around addUserToRoomSlashCommand at $line ====="
sed -n "${start},${end}p" "$file"
doneRepository: RocketChat/Rocket.Chat
Length of output: 7233
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for route handlers for channels.invite / groups.invite and see expected body fields
rg -n "channels\\.invite|groups\\.invite" apps/meteor/app apps/meteor/server apps/meteor/lib apps/meteor -S --hidden --glob '!**/node_modules/**' | head -n 50Repository: RocketChat/Rocket.Chat
Length of output: 4878
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Narrow to API route definitions (common pattern: 'channels.invite' string)
rg -n "\"channels\\.invite\"|\"groups\\.invite\"" apps/meteor --glob '*.{ts,js}' | head -n 50Repository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find specific files likely handling invites and then inspect for username usage
files=$(rg -l "\"channels\\.invite\"|\"groups\\.invite\"" apps/meteor --glob '*.{ts,js}')
echo "$files" | head -n 20Repository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '90,240p' apps/meteor/tests/data/rooms.helper.tsRepository: RocketChat/Rocket.Chat
Length of output: 4425
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Inspect rooms.helper.ts around the referenced regions
sed -n '90,240p' apps/meteor/tests/data/rooms.helper.tsRepository: RocketChat/Rocket.Chat
Length of output: 4425
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 2) Inspect how addUserToRoom is imported/used in federation-matrix tests and a couple other callers
for f in \
ee/packages/federation-matrix/tests/end-to-end/permissions.spec.ts \
ee/packages/federation-matrix/tests/end-to-end/dms.spec.ts \
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts \
apps/meteor/tests/end-to-end/api/chat.ts
do
echo "===== $f (addUserToRoom occurrences) ====="
rg -n "addUserToRoom" "$f" | head -n 30
echo
echo "===== $f (around first occurrence) ====="
line=$(rg -n "addUserToRoom" "$f" | head -n 1 | cut -d: -f1)
start=$((line-15)); end=$((line+35))
sed -n "${start},${end}p" "$f"
echo
doneRepository: RocketChat/Rocket.Chat
Length of output: 2695
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 3) Inspect slash-command usage in room.spec.ts
file="ee/packages/federation-matrix/tests/end-to-end/room.spec.ts"
rg -n "addUserToRoomSlashCommand|commands\\.run\\(|response\\.body\\.message|msg:'result'|room\\.spec\\.ts" "$file" | head -n 120
for line in $(rg -n "addUserToRoomSlashCommand" "$file" | cut -d: -f1 | head -n 10); do
start=$((line-25)); end=$((line+35))
echo "===== $file around addUserToRoomSlashCommand at $line ====="
sed -n "${start},${end}p" "$file"
doneRepository: RocketChat/Rocket.Chat
Length of output: 2380
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 4) Locate API handlers for channels.invite / groups.invite
rg -n "\"channels\\.invite\"|\"groups\\.invite\"" apps/meteor --glob '*.{ts,js}' | head -n 50Repository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 5) List candidate files implementing those handlers and inspect for 'username' usage
files=$(rg -l "\"channels\\.invite\"|\"groups\\.invite\"" apps/meteor --glob '*.{ts,js}' | head -n 20)
echo "$files"Repository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 5b) If we found candidate files, print a small window around each match for 'username'
files=$(rg -l "\"channels\\.invite\"|\"groups\\.invite\"" apps/meteor --glob '*.{ts,js}' | head -n 20)
for f in $files; do
echo "===== $f matches (channels.invite/groups.invite) ====="
rg -n "\"channels\\.invite\"|\"groups\\.invite\"" "$f" | head -n 5
echo "===== $f around match ====="
line=$(rg -n "\"channels\\.invite\"|\"groups\\.invite\"" "$f" | head -n 1 | cut -d: -f1)
start=$((line-20)); end=$((line+60))
sed -n "${start},${end}p" "$f"
echo
doneRepository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="ee/packages/federation-matrix/tests/end-to-end/permissions.spec.ts"
# Extract around the first few addUserToRoom calls (use line numbers from previous rg output)
# We'll print a wider window around each known line: 183,195,266,419,514,597,661,870
for line in 183 195 266 419 514 597 661 870; do
start=$((line-15)); end=$((line+25))
echo "===== $file around line $line ====="
awk -v s=$start -v e=$end 'NR>=s && NR<=e {printf "%5d:%s\n", NR, $0}' "$file"
echo
doneRepository: RocketChat/Rocket.Chat
Length of output: 12331
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="ee/packages/federation-matrix/tests/end-to-end/dms.spec.ts"
for line in 691 1177 1743 1789; do
start=$((line-15)); end=$((line+25))
echo "===== $file around line $line ====="
awk -v s=$start -v e=$end 'NR>=s && NR<=e {printf "%5d:%s\n", NR, $0}' "$file"
echo
doneRepository: RocketChat/Rocket.Chat
Length of output: 7109
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="ee/packages/federation-matrix/tests/end-to-end/room.spec.ts"
# Extract around the /invite slash-command parsing lines we saw: 259-293 and 278
for line in 230 250 259 278 1226 1259 1362; do
start=$((line-15)); end=$((line+35))
echo "===== $file around line $line ====="
awk -v s=$start -v e=$end 'NR>=s && NR<=e {printf "%5d:%s\n", NR, $0}' "$file"
echo
doneRepository: RocketChat/Rocket.Chat
Length of output: 17861
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find server handlers /v1/channels.invite and /v1/groups.invite
# First list candidate files containing those exact route strings.
rg -n --glob '*.{ts,js}' '\/v1\/(channels\.invite|groups\.invite)' apps/meteor --hidden | head -n 50Repository: RocketChat/Rocket.Chat
Length of output: 172
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# If previous search isn't enough, search for "channels.invite" / "groups.invite" route constants in REST API code
rg -n --glob '*.{ts,js}' '"channels\.invite"|"groups\.invite"' apps/meteor/app apps/meteor/server apps/meteor/lib --hidden | head -n 80Repository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect commands.run endpoint implementation to understand response shape
rg -n --glob '*.{ts,js}' "commands\\.run" apps/meteor | head -n 50Repository: RocketChat/Rocket.Chat
Length of output: 2935
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Print code around commands.run handler if found
# Try common patterns: /api/v1/commands.run or 'commands/run' etc.
rg -n --glob '*.{ts,js}' "(api/v1/commands\\.run|commands\\.run.*handler|commands\\.run)" apps/meteor --hidden | head -n 20Repository: RocketChat/Rocket.Chat
Length of output: 1816
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="ee/packages/federation-matrix/tests/end-to-end/permissions.spec.ts"
# Extract around the first few addUserToRoom calls (line numbers from previous rg output)
for line in 183 195 266 419 514 597 661 870; do
start=$((line-15)); end=$((line+25))
echo "===== $file around line $line ====="
awk -v s=$start -v e=$end 'NR>=s && NR<=e {printf "%5d:%s\n", NR, $0}' "$file"
echo
doneRepository: RocketChat/Rocket.Chat
Length of output: 12331
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="ee/packages/federation-matrix/tests/end-to-end/dms.spec.ts"
for line in 691 1177 1743 1789; do
start=$((line-15)); end=$((line+25))
echo "===== $file around line $line ====="
awk -v s=$start -v e=$end 'NR>=s && NR<=e {printf "%5d:%s\n", NR, $0}' "$file"
echo
doneRepository: RocketChat/Rocket.Chat
Length of output: 7109
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="ee/packages/federation-matrix/tests/end-to-end/room.spec.ts"
for line in 230 250 259 278 1226 1259 1362; do
start=$((line-15)); end=$((line+35))
echo "===== $file around line $line ====="
awk -v s=$start -v e=$end 'NR>=s && NR<=e {printf "%5d:%s\n", NR, $0}' "$file"
echo
doneRepository: RocketChat/Rocket.Chat
Length of output: 17861
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --glob '*.{ts,js}' '\/v1\/(channels\.invite|groups\.invite)' apps/meteor --hidden | head -n 50Repository: RocketChat/Rocket.Chat
Length of output: 172
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --glob '*.{ts,js}' '"channels\.invite"|"groups\.invite"' apps/meteor/app apps/meteor/server apps/meteor/lib --hidden | head -n 80Repository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --glob '*.{ts,js}' "commands\.run" apps/meteor | head -n 50Repository: RocketChat/Rocket.Chat
Length of output: 2935
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="ee/packages/federation-matrix/tests/end-to-end/permissions.spec.ts"
# Extract around the first few addUserToRoom calls (line numbers from previous rg output)
for line in 183 195 266 419 514 597 661 870; do
start=$((line-15)); end=$((line+25))
echo "===== $file around line $line ====="
awk -v s=$start -v e=$end 'NR>=s && NR<=e {printf "%5d:%s\n", NR, $0}' "$file"
echo
doneRepository: RocketChat/Rocket.Chat
Length of output: 12331
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="ee/packages/federation-matrix/tests/end-to-end/dms.spec.ts"
for line in 691 1177 1743 1789; do
start=$((line-15)); end=$((line+25))
echo "===== $file around line $line ====="
awk -v s=$start -v e=$end 'NR>=s && NR<=e {printf "%5d:%s\n", NR, $0}' "$file"
echo
doneRepository: RocketChat/Rocket.Chat
Length of output: 7109
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="ee/packages/federation-matrix/tests/end-to-end/room.spec.ts"
for line in 230 250 259 278 1226 1259 1362; do
start=$((line-15)); end=$((line+35))
echo "===== $file around line $line ====="
awk -v s=$start -v e=$end 'NR>=s && NR<=e {printf "%5d:%s\n", NR, $0}' "$file"
echo
doneRepository: RocketChat/Rocket.Chat
Length of output: 17861
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --glob '*.{ts,js}' '\/v1\/(channels\.invite|groups\.invite)' apps/meteor --hidden | head -n 50Repository: RocketChat/Rocket.Chat
Length of output: 172
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --glob '*.{ts,js}' '"channels\.invite"|"groups\.invite"' apps/meteor/app apps/meteor/server apps/meteor/lib --hidden | head -n 80Repository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --glob '*.{ts,js}' "commands\.run" apps/meteor | head -n 50Repository: RocketChat/Rocket.Chat
Length of output: 2935
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Check whether federation-matrix specs destructure addUserToRoom return value
for f in \
ee/packages/federation-matrix/tests/end-to-end/permissions.spec.ts \
ee/packages/federation-matrix/tests/end-to-end/dms.spec.ts \
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
do
echo "===== $f destructuring addUserToRoom ====="
rg -n "const \\[.*\\].*= *await addUserToRoom" "$f" || true
rg -n "await addUserToRoom\\(" "$f" || true
echo
doneRepository: RocketChat/Rocket.Chat
Length of output: 1273
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 2) Find API route handlers for /v1/channels.invite and /v1/groups.invite
# First, list occurrences of those route strings in the server API code
rg -n "\"channels\\.invite\"|\"groups\\.invite\"" apps/meteor/app apps/meteor/server apps/meteor/lib --glob '*.{ts,js}' --hidden | head -n 50Repository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 3) Also search for 'channels.invite' / 'groups.invite' without quotes
rg -n "channels\\.invite|groups\\.invite" apps/meteor/app apps/meteor/server apps/meteor/lib --glob '*.{ts,js}' --hidden | head -n 50Repository: RocketChat/Rocket.Chat
Length of output: 519
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 4) Inspect the invite controller/service if we can locate it
# Try to locate addUserToRoom / invite usage patterns
# We look for "req.body.*username" and "req.body.*userId" near invite endpoints
rg -n "invite.*(username|userId)" apps/meteor/app apps/meteor/server apps/meteor/lib --glob '*.{ts,js}' --hidden | head -n 80Repository: RocketChat/Rocket.Chat
Length of output: 2800
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 5) Specifically inspect likely handler files for invite: channels/groups api endpoints.
# We'll search for function names or modules: "channels.invite" may be defined in routes.
candidates=$(rg -l "\"channels\\.invite\"|\"groups\\.invite\"" apps/meteor/app apps/meteor/server apps/meteor/lib --glob '*.{ts,js}' --hidden | tr '\n' ' ')
echo "Candidates: $candidates" | head -n 1Repository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 6) If we have candidates, print the relevant sections around first match in each
# (Only for up to 3 files to keep output small)
files=$(rg -l "\"channels\\.invite\"|\"groups\\.invite\"" apps/meteor/app apps/meteor/server apps/meteor/lib --glob '*.{ts,js}' --hidden | head -n 3)
for f in $files; do
echo "===== $f matches ====="
line=$(rg -n "\"channels\\.invite\"|\"groups\\.invite\"" "$f" | head -n 1 | cut -d: -f1)
start=$((line-40)); end=$((line+80))
echo "around line $line:"
awk -v s=$start -v e=$end 'NR>=s && NR<=e {printf "%5d:%s\n", NR, $0}' "$f"
echo
doneRepository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Check whether federation-matrix specs destructure addUserToRoom return value
for f in \
ee/packages/federation-matrix/tests/end-to-end/permissions.spec.ts \
ee/packages/federation-matrix/tests/end-to-end/dms.spec.ts \
ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
do
echo "===== $f destructuring addUserToRoom ====="
rg -n "const \\[.*\\].*= *await addUserToRoom" "$f" || true
rg -n "await addUserToRoom\\(" "$f" || true
echo
doneRepository: RocketChat/Rocket.Chat
Length of output: 1273
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 2) Find API route handlers for /v1/channels.invite and /v1/groups.invite
rg -n "\"channels\\.invite\"|\"groups\\.invite\"" apps/meteor/app apps/meteor/server apps/meteor/lib --glob '*.{ts,js}' --hidden | head -n 50Repository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 3) Also search for 'channels.invite' / 'groups.invite' without quotes
rg -n "channels\\.invite|groups\\.invite" apps/meteor/app apps/meteor/server apps/meteor/lib --glob '*.{ts,js}' --hidden | head -n 50Repository: RocketChat/Rocket.Chat
Length of output: 519
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 4) Search for invite handlers reading either username or userId
rg -n "invite.*(username|userId)" apps/meteor/app apps/meteor/server apps/meteor/lib --glob '*.{ts,js}' --hidden | head -n 80Repository: RocketChat/Rocket.Chat
Length of output: 2800
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 5) Print the sections around the first match for up to 3 located files
files=$(rg -l "\"channels\\.invite\"|\"groups\\.invite\"" apps/meteor/app apps/meteor/server apps/meteor/lib --glob '*.{ts,js}' --hidden | head -n 3)
for f in $files; do
echo "===== $f matches ====="
line=$(rg -n "\"channels\\.invite\"|\"groups\\.invite\"" "$f" | head -n 1 | cut -d: -f1)
start=$((line-40)); end=$((line+120))
awk -v s=$start -v e=$end 'NR>=s && NR<=e {printf "%5d:%s\n", NR, $0}' "$f"
echo
doneRepository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect invite handlers in channels/groups API files around the discovered route registration lines
echo "=== apps/meteor/app/api/server/v1/channels.ts around 'channels.invite' ==="
file="apps/meteor/app/api/server/v1/channels.ts"
line=$(rg -n "'channels\\.invite'" "$file" | head -n 1 | cut -d: -f1)
start=$((line-80)); end=$((line+120))
awk -v s=$start -v e=$end 'NR>=s && NR<=e {printf "%5d:%s\n", NR, $0}' "$file"
echo
echo "=== apps/meteor/app/api/server/v1/groups.ts around 'groups.invite' ==="
file="apps/meteor/app/api/server/v1/groups.ts"
line=$(rg -n "'groups\\.invite'" "$file" | head -n 1 | cut -d: -f1)
start=$((line-80)); end=$((line+120))
awk -v s=$start -v e=$end 'NR>=s && NR<=e {printf "%5d:%s\n", NR, $0}' "$file"Repository: RocketChat/Rocket.Chat
Length of output: 12578
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the channels/groups API route implementation to find request body parsing
# Specifically search within those files for 'req.body', 'username', 'userId', 'roomId'
for file in apps/meteor/app/api/server/v1/channels.ts apps/meteor/app/api/server/v1/groups.ts; do
echo "=== body parsing in $file ==="
rg -n "channels\\.invite|groups\\.invite|req\\.body|username|userId|roomId" "$file" | head -n 120
echo
doneRepository: RocketChat/Rocket.Chat
Length of output: 16040
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect /v1/commands.run implementation to see what it returns in body.message
file="apps/meteor/app/api/server/v1/commands.ts"
line=$(rg -n "commands\\.run" "$file" | head -n 1 | cut -d: -f1)
start=$((line-120)); end=$((line+220))
awk -v s=$start -v e=$end 'NR>=s && NR<=e {printf "%5d:%s\n", NR, $0}' "$file"Repository: RocketChat/Rocket.Chat
Length of output: 8689
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "v1\\.success\\(|success\\(\\{?\\s*.*\\)\\s*\\{|class API|API\\.v1" apps/meteor/app/api/server --hidden --glob '*.{ts,js}' | head -n 50Repository: RocketChat/Rocket.Chat
Length of output: 4357
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Try to locate the API helper source directly
rg -n "static success|success\\(data\\)|success\\(payload\\)" apps/meteor/app --hidden --glob '*.{ts,js}' | head -n 50Repository: RocketChat/Rocket.Chat
Length of output: 234
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Narrow to common API implementation file names
for f in $(fd -t f "api.*\\.ts" apps/meteor/app -a 2>/dev/null | head -n 20); do
echo "FILE $f"
rg -n "success\\(" "$f" | head -n 5 || true
doneRepository: RocketChat/Rocket.Chat
Length of output: 1281
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="apps/meteor/app/api/server/ApiClass.ts"
# show around the success() implementation
rg -n "success\\(" "$file" | head -n 20Repository: RocketChat/Rocket.Chat
Length of output: 143
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="apps/meteor/app/api/server/ApiClass.ts"
# Print a focused window around ApiClass.success implementation by locating the first occurrence of "public success"
line=$(rg -n "public success" "$file" | head -n 1 | cut -d: -f1)
start=$((line-80)); end=$((line+120))
awk -v s=$start -v e=$end 'NR>=s && NR<=e {printf "%5d:%s\n", NR, $0}' "$file"Repository: RocketChat/Rocket.Chat
Length of output: 6206
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find all imports of addUserToRoom from rooms.helper.ts
rg -n "from ['\"].*apps/meteor/tests/data/rooms\.helper['\"]|from ['\"].*rooms\.helper['\"]" . --glob '*.{ts,tsx,js,jsx}' --hidden \
| rg -n "rooms\.helper|addUserToRoom" \
| head -n 200Repository: RocketChat/Rocket.Chat
Length of output: 4317
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find all call sites of the *imported helper* by matching the property access pattern.
# This is less precise than import analysis, but we'll constrain by looking for:
# addUserToRoom({ ... usernames: [...] ... })
# and then inspect nearby code for `type:` or destructuring.
rg -n "addUserToRoom\\s*\\(\\s*\\{" ee apps/meteor/tests --glob '*.{ts,tsx}' --hidden \
| head -n 200Repository: RocketChat/Rocket.Chat
Length of output: 2127
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# For the call sites, check within a small context window for `type:` presence.
# Report line numbers where addUserToRoom is called but no `type:` appears near it.
# (Heuristic; mainly to catch missing type in TS callers.)
rg -n "addUserToRoom\\s*\\(\\s*\\{" ee packages apps/meteor/tests --glob '*.{ts,tsx}' --hidden > /tmp/addUserToRoom_calls.txt
while IFS=: read -r file line rest; do
start=$((line-5)); end=$((line+15))
window=$(awk -v s=$start -v e=$end 'NR>=s && NR<=e {print}' "$file")
if ! echo "$window" | rg -q "\btype\s*:"; then
echo "$file:$line"
fi
done < /tmp/addUserToRoom_calls.txt | head -n 100Repository: RocketChat/Rocket.Chat
Length of output: 986
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show addUserToRoom usage in the two non-federation-matrix call sites
awk 'NR>=2335 && NR<=2375 {print NR ":" $0}' apps/meteor/tests/end-to-end/api/chat.ts
echo "----"
awk 'NR>=300 && NR<=360 {print NR ":" $0}' apps/meteor/tests/data/uploads.helper.tsRepository: RocketChat/Rocket.Chat
Length of output: 3932
Fix breaking helper contract inconsistencies for addUserToRoom and /invite REST responses
apps/meteor/tests/data/rooms.helper.ts(addUserToRoom) now requirestype: 'c' | 'p'and returnsPromise.all(...)(array), but federation-matrix callers likeee/packages/federation-matrix/tests/end-to-end/permissions.spec.tsand.../dms.spec.tscall it withouttypeand treat the return value as a single response (addUserResponse.status/response.body...) instead of indexing/destructuring[response].ee/packages/federation-matrix/tests/end-to-end/room.spec.tsparsesaddUserToRoomSlashCommandas ifcommands.runreturnsresponse.body.message, but the server routecommands.runreturnsAPI.v1.success({ result })(i.e.,response.body.result), so the test parsing needs to align with that shape.- The
addUserToRoomJSDoc is stale (mentions the old DDP/addUsersToRoom behavior and omits the new requiredtypeparameter).
🤖 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 `@apps/meteor/tests/data/rooms.helper.ts` around lines 132 - 166, The
addUserToRoom helper now breaks callers by requiring type and always returning
an array; make the helper backward-compatible by making the type parameter
optional (default to 'c' when missing), update the return shape to return the
single request response when usernames.length === 1 and an array only when
multiple usernames are provided, and update the JSDoc for addUserToRoom to
document the optional type and return shape; also fix the affected
federation-matrix tests (e.g., permissions.spec.ts, dms.spec.ts, room.spec.ts)
that parse commands.run responses to read response.body.result (not
response.body.message) so they match the server route that returns
API.v1.success({ result }).
Comment on lines
202
to
211
| return requestInstance | ||
| .post(methodCall('slashCommand')) | ||
| .post(api('commands.run')) | ||
| .set(credentialsInstance) | ||
| .send({ | ||
| message: JSON.stringify({ | ||
| method: 'slashCommand', | ||
| params: [ | ||
| { | ||
| cmd: 'invite', | ||
| params: usernames.join(' '), | ||
| msg: { | ||
| rid, | ||
| _id: `test-${Date.now()}`, | ||
| }, | ||
| triggerId: `test-trigger-${Date.now()}`, | ||
| }, | ||
| ], | ||
| id: 'id', | ||
| msg: 'method', | ||
| }), | ||
| command: 'invite', | ||
| params: usernames.join(' '), | ||
| roomId: rid, | ||
| triggerId: `test-trigger-${Date.now()}`, | ||
| }); | ||
| }; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm room.spec.ts still expects a DDP `message` envelope from addUserToRoomSlashCommand...
fd 'room.spec.ts' | xargs rg -nP -C6 'addUserToRoomSlashCommand|response\.body\.message'
# ...and inspect the commands.run REST endpoint response shape.
rg -nP -C12 "'/v1/commands.run'|addEndpoint\(\s*'commands.run'"Repository: RocketChat/Rocket.Chat
Length of output: 13489
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Inspect the helper implementation around the referenced lines
echo "---- apps/meteor/tests/data/rooms.helper.ts (lines ~180-240) ----"
sed -n '180,240p' apps/meteor/tests/data/rooms.helper.ts
# 2) Find the server implementation / route wiring for /v1/commands.run
echo
echo "---- Search for commands.run route/handler implementation ----"
rg -n --hidden --no-ignore \
"(addEndpoint\\(.*'commands\\.run'|/v1/commands\\.run|commands\\.run')" \
apps packages/rest-typings packages/rest-implementation packages/meteor 2>/dev/null || true
# 3) Narrow to server code under apps/meteor if present
echo
echo "---- Search specifically under apps/meteor for commands.run ----"
rg -n "commands\\.run" apps/meteor 2>/dev/null || true
# 4) Inspect the federation-matrix test assertions context (room.spec.ts)
echo
echo "---- ee/packages/federation-matrix/tests/end-to-end/room.spec.ts (lines ~240-310) ----"
sed -n '240,310p' ee/packages/federation-matrix/tests/end-to-end/room.spec.tsRepository: RocketChat/Rocket.Chat
Length of output: 10464
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Inspect /v1/commands.run handler implementation in apps/meteor
echo "---- apps/meteor/app/api/server/v1/commands.ts around commands.run registration ----"
sed -n '330,460p' apps/meteor/app/api/server/v1/commands.ts
# 2) Locate the exact handler function for commands.run (named/anonymous) and print its return shape
echo
echo "---- Search within commands.ts for 'commands.run' handler function / implementation ----"
rg -n "commands\\.run|/v1/commands\\.run" apps/meteor/app/api/server/v1/commands.ts
# 3) Inspect the server return object fields (especially 'message') near the handler
# (Use a wider window around the matches)
echo
echo "---- Print larger window around the commands.run handler ----"
rg -n "commands\\.run" -n apps/meteor/app/api/server/v1/commands.ts | head -n 20 | awk -F: '{print $1}' | head -n 1 >/tmp/match_line.txt
LINE=$(cat /tmp/match_line.txt)
START=$((LINE-120))
END=$((LINE+200))
sed -n "${START},${END}p" apps/meteor/app/api/server/v1/commands.ts
# 4) Inspect rest-typings return type for /v1/commands.run to confirm whether 'message' exists
echo
echo "---- packages/rest-typings/src/v1/commands.ts ----"
sed -n '1,120p' packages/rest-typings/src/v1/commands.tsRepository: RocketChat/Rocket.Chat
Length of output: 11411
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect existing /commands.run endpoint tests for expectations about response fields (message vs result)
echo "---- apps/meteor/tests/end-to-end/api/commands.ts (commands.run assertions) ----"
rg -n "commands\\.run" apps/meteor/tests/end-to-end/api/commands.ts
echo
# Show a representative window near the first commands.run test block
MATCH_LINE=$(rg -n "describe\\('\\[/commands\\.run\\]" apps/meteor/tests/end-to-end/api/commands.ts | head -n 1 | cut -d: -f1)
START=$((MATCH_LINE-40))
END=$((MATCH_LINE+240))
sed -n "${START},${END}p" apps/meteor/tests/end-to-end/api/commands.ts
# Search within that test file for ".body.message" usage
echo
echo "---- Search for response.body.message in apps/meteor/tests/end-to-end/api/commands.ts ----"
rg -n "body\\.message" apps/meteor/tests/end-to-end/api/commands.ts || true
# Inspect API.v1.success implementation to see exact success payload fields
echo
echo "---- Search for API.v1.success definition ----"
rg -n "v1\\.success\\(" apps/meteor packages/rest-implementation apps/packages 2>/dev/null || true
rg -n "API\\.v1\\.success" . | head -n 20Repository: RocketChat/Rocket.Chat
Length of output: 50378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Locate the implementation of API.v1.success (formatting/wrapping of response bodies)
echo "---- Locate API.v1.success implementation ----"
rg -n "API\\.v1\\.success\\s*=|function\\s+success\\(|v1\\.success\\s*\\(" apps/meteor/app packages/rest-implementation packages/rest-typings packages -S -g'*.ts' -g'*.js' 2>/dev/null | head -n 50
# Try a more targeted search for the API object itself
echo
echo "---- Search for 'class API' / 'const API' definition ----"
rg -n "class\\s+API\\b|const\\s+API\\b|export\\s+(class|const)\\s+API\\b" apps/meteor/app packages/rest-implementation packages -S -g'*.ts' 2>/dev/null | head -n 50
# 2) Once found, print the relevant portion around API.v1.success
# (We use a fallback glob to print likely candidates if paths are found.)
echo
echo "---- Candidate files containing 'API.v1.success' ----"
rg -n "API\\.v1\\.success" apps/meteor/app/api apps/meteor/app/livechat/server packages/rest-implementation -S -g'*.ts' 2>/dev/null | head -n 30
# 3) If the API implementation file is identified by name in results, we’ll print it next turn.Repository: RocketChat/Rocket.Chat
Length of output: 4785
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find where API.v1 is defined (rest implementation)
echo "---- Find API.v1 definition / API object ----"
rg -n --no-ignore --hidden "API\\.v1\\s*=" apps/meteor/app packages/rest-implementation -S -g'*.ts' -g'*.js' 2>/dev/null || true
# Look for a success helper definition in rest-implementation
echo
echo "---- Find API.v1.success implementation candidates ----"
rg -n --no-ignore --hidden "success\\s*\\(.*\\)\\s*\\{|function\\s+success\\b|\\bv1\\.success\\b" apps/meteor/app packages/rest-implementation -S -g'*.ts' -g'*.js' 2>/dev/null | head -n 50
# If not found, search for "API.v1.failure" alongside it
echo
echo "---- Find API.v1.failure definition candidates (paired with success) ----"
rg -n --no-ignore --hidden "API\\.v1\\.failure|v1\\.failure\\b|failure\\s*\\(.*\\)\\s*\\{" apps/meteor/app packages/rest-implementation -S -g'*.ts' -g'*.js' 2>/dev/null | head -n 50
# Search in the generic rest API utility module (often named api.ts)
echo
echo "---- Search for 'class API' or 'const API' within rest-implementation ----"
rg -n --no-ignore --hidden "class\\s+API\\b|const\\s+API\\b|export\\s+const\\s+API\\b" packages/rest-implementation apps/meteor/app -S -g'*.ts' 2>/dev/null | head -n 50Repository: RocketChat/Rocket.Chat
Length of output: 5119
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect imports/definition source for `API` in commands.run handler
echo "---- apps/meteor/app/api/server/v1/commands.ts imports (top) ----"
sed -n '1,80p' apps/meteor/app/api/server/v1/commands.ts
# Extract likely API import paths from the file
echo
echo "---- Search for 'from .*api' imports around API usage ----"
rg -n "from ['\"][^'\"]*api[^'\"]*['\"]|from ['\"][^'\"]*rest[^'\"]*['\"]" apps/meteor/app/api/server/v1/commands.ts
# Locate the actual module exporting `API` by searching for `export.*API` near those import paths
# (We don't know the exact path yet; we do a broad search for API.v1.post/success patterns.)
echo
echo "---- Search for definition of API.v1.post in codebase ----"
rg -n "v1\\.post\\(|API\\.v1\\.post\\(" apps/meteor/app packages/rest-implementation -S -g'*.ts' 2>/dev/null | head -n 50
echo
echo "---- Search for API.success signature implementation (generic) ----"
rg -n "success\\s*\\(.*\\)\\s*\\{" packages/rest-implementation packages/rest-typings apps/meteor/app -S -g'*.ts' 2>/dev/null | head -n 50
# Also search for the exact string used in this file
echo
echo "---- Search for 'API\\.v1\\.success' definition (not just usages) ----"
rg -n "export (const|function) success\\b|function success\\b|success\\s*:\\s*\\(" packages/rest-implementation apps/meteor/app -S -g'*.ts' 2>/dev/null | head -n 50Repository: RocketChat/Rocket.Chat
Length of output: 5950
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the API module imported as '../api' from apps/meteor/app/api/server/v1/commands.ts
echo "---- Resolve ../api path targets (expect apps/meteor/app/api/server/api.*) ----"
ls -la apps/meteor/app/api/server | head
ls -la apps/meteor/app/api/server/api* 2>/dev/null || true
ls -la apps/meteor/app/api/server/*api* 2>/dev/null | head -n 50 || true
# Search for the exported API symbol definition near that module
echo
echo "---- Search for export of API in apps/meteor/app/api/server ----"
rg -n "export\\s+(const|class)\\s+API\\b|export\\s+\\{\\s*API\\s*\\}" apps/meteor/app/api/server -S -g'*.ts' || true
# Inspect the API module that defines API.v1.success
echo
echo "---- Search for 'v1.success' or 'success(' in apps/meteor/app/api/server/api* ----"
rg -n "v1\\.success|API\\.v1\\.success|success\\s*\\(" apps/meteor/app/api/server -S -g'api*.ts' -g'*.ts' || true
# Once we find it, print the relevant portion.
echo
echo "---- Candidate files containing 'API.v1' ----"
rg -n "API\\.v1" apps/meteor/app/api/server -S -g'*.ts' | head -n 50Repository: RocketChat/Rocket.Chat
Length of output: 36458
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the ApiClass success implementation that builds the response payload
echo "---- apps/meteor/app/api/server/ApiClass.ts (SuccessResult + success helper) ----"
rg -n "type\\s+SuccessResult|public\\s+success\\(|success\\(payload" apps/meteor/app/api/server/ApiClass.ts
echo
# Print a window around the first success method we find
LINE=$(rg -n "public success\\(" apps/meteor/app/api/server/ApiClass.ts | head -n 1 | cut -d: -f1)
START=$((LINE-40))
END=$((LINE+120))
sed -n "${START},${END}p" apps/meteor/app/api/server/ApiClass.ts
# Also print the API wrapper that is used by commands.ts (apps/meteor/app/api/server/api.ts)
echo
echo "---- apps/meteor/app/api/server/api.ts around export const API definition ----"
sed -n '1,120p' apps/meteor/app/api/server/api.tsRepository: RocketChat/Rocket.Chat
Length of output: 7980
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect slashCommands.run implementation/return shape
echo "---- apps/meteor/app/utils/server/slashCommand.ts (search slashCommands.run definition) ----"
rg -n "slashCommands\\.run|run\\s*\\(" apps/meteor/app/utils/server/slashCommand.ts
echo
echo "---- Print region around slashCommands.run ----"
rg -n "slashCommands\\.run" apps/meteor/app/utils/server/slashCommand.ts | head -n 1 | cut -d: -f1 > /tmp/line.txt
LINE=$(cat /tmp/line.txt)
START=$((LINE-80))
END=$((LINE+160))
sed -n "${START},${END}p" apps/meteor/app/utils/server/slashCommand.tsRepository: RocketChat/Rocket.Chat
Length of output: 2710
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "response\\.body\\.message|\\.body\\.message" ee/packages/federation-matrix/tests/end-to-end/room.spec.tsRepository: RocketChat/Rocket.Chat
Length of output: 130
Update the federation slash-command test assertion: REST /v1/commands.run doesn’t return a DDP message envelope.
apps/meteor/tests/data/rooms.helper.ts posts the invite via REST api('commands.run'), but ee/packages/federation-matrix/tests/end-to-end/room.spec.ts still parses response.body.message as JSON and asserts msg: 'result'. The REST endpoint implementation (apps/meteor/app/api/server/v1/commands.ts /v1/commands.run) returns API.v1.success({ result }), so the response body is { success: true, result: ... } (no message field), making JSON.parse(response.body.message) invalid.
Either keep a DDP-based helper/flow for this assertion or change room.spec.ts to assert on response.body.result instead.
🤖 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 `@apps/meteor/tests/data/rooms.helper.ts` around lines 202 - 211, The REST
helper in rooms.helper.ts posts to api('commands.run') which returns
API.v1.success({ result }) (no DDP `message` envelope), so the federation test
in ee/packages/federation-matrix/tests/end-to-end/room.spec.ts must stop parsing
response.body.message; update the test to assert against response.body.result
(or switch to a DDP-based helper), specifically replace
JSON.parse(response.body.message) and checks for msg:'result' with direct
assertions on response.body.result to match the /v1/commands.run implementation.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
2 issues found across 4 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="apps/meteor/tests/data/rooms.helper.ts">
<violation number="1" location="apps/meteor/tests/data/rooms.helper.ts:158">
P1: `addUserToRoom` now returns an array of per-user REST responses, breaking existing callers that still expect one response object and changing multi-user invite semantics to partial-success-per-user.</violation>
<violation number="2" location="apps/meteor/tests/data/rooms.helper.ts:203">
P1: Switching `addUserToRoomSlashCommand` from DDP (`methodCall('slashCommand')`) to REST (`api('commands.run')`) changes the response shape from a DDP envelope (`{ message: '{"msg":"result",...}' }`) to the standard REST envelope (`{ success: true }` or `{ success: false, error: '...' }`). Consumers in `room.spec.ts` still call `JSON.parse(response.body.message)` which will throw since `response.body.message` is `undefined` with the REST endpoint.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| msg: 'method', | ||
| const endpoint = type === 'c' ? 'channels.invite' : 'groups.invite'; | ||
|
|
||
| return Promise.all( |
Contributor
There was a problem hiding this comment.
P1: addUserToRoom now returns an array of per-user REST responses, breaking existing callers that still expect one response object and changing multi-user invite semantics to partial-success-per-user.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/tests/data/rooms.helper.ts, line 158:
<comment>`addUserToRoom` now returns an array of per-user REST responses, breaking existing callers that still expect one response object and changing multi-user invite semantics to partial-success-per-user.</comment>
<file context>
@@ -129,55 +129,56 @@ export const getSubscriptionByRoomId = (roomId: IRoom['_id'], userCredentials =
- msg: 'method',
+ const endpoint = type === 'c' ? 'channels.invite' : 'groups.invite';
+
+ return Promise.all(
+ usernames.map((username) =>
+ requestInstance.post(api(endpoint)).set(credentialsInstance).send({
</file context>
|
|
||
| return requestInstance | ||
| .post(methodCall('slashCommand')) | ||
| .post(api('commands.run')) |
Contributor
There was a problem hiding this comment.
P1: Switching addUserToRoomSlashCommand from DDP (methodCall('slashCommand')) to REST (api('commands.run')) changes the response shape from a DDP envelope ({ message: '{"msg":"result",...}' }) to the standard REST envelope ({ success: true } or { success: false, error: '...' }). Consumers in room.spec.ts still call JSON.parse(response.body.message) which will throw since response.body.message is undefined with the REST endpoint.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/tests/data/rooms.helper.ts, line 203:
<comment>Switching `addUserToRoomSlashCommand` from DDP (`methodCall('slashCommand')`) to REST (`api('commands.run')`) changes the response shape from a DDP envelope (`{ message: '{"msg":"result",...}' }`) to the standard REST envelope (`{ success: true }` or `{ success: false, error: '...' }`). Consumers in `room.spec.ts` still call `JSON.parse(response.body.message)` which will throw since `response.body.message` is `undefined` with the REST endpoint.</comment>
<file context>
@@ -198,25 +200,13 @@ export const addUserToRoomSlashCommand = ({
return requestInstance
- .post(methodCall('slashCommand'))
+ .post(api('commands.run'))
.set(credentialsInstance)
.send({
</file context>
Author
|
This PR migrates federation add-user tests from legacy DDP response assertions to REST response envelopes as part of the DDP removal cleanup. I’ve verified:
Could someone please review and approve the workflows when available? Thanks! |
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.
Proposed changes (including videos or screenshots)
fixes: #40709
This PR migrates federation add-user tests from the legacy DDP-style response assertions to the newer REST response envelope.
Changes made
Updated
addUserToRoomhelper inapps/meteor/tests/data/rooms.helper.tsMigrated from:
/api/v1/method.call/addUsersToRoomTo:
/api/v1/channels.invite/api/v1/groups.inviteUpdated federation end-to-end tests to assert REST response structures instead of DDP
messagepayloads.Updated assertions
Changed checks like:
and:
to REST-style assertions such as:
and:
Files updated
apps/meteor/tests/data/rooms.helper.tsee/packages/federation-matrix/tests/end-to-end/permissions.spec.tsee/packages/federation-matrix/tests/end-to-end/dms.spec.tsThe remaining usage in
room.spec.tswas intentionally left unchanged because the slash-command flow still uses DDP ephemeral messaging behavior.Issue(s)
Related to the DDP removal migration TODOs for federation invite flows.
Steps to test or reproduce
Run federation matrix end-to-end tests:
Verify:
successanderrorfieldsRun lint checks:
Further comments
This PR only migrates the direct add-user helper and related federation assertions to REST responses.
The slash-command flow (
addUserToRoomSlashCommand) was intentionally not migrated because it still depends on DDP ephemeral message broadcasting behavior and does not yet have equivalent REST-based handling.Summary by CodeRabbit