Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/migrate-chat-postMessage-to-AJV.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@rocket.chat/rest-typings': minor
'@rocket.chat/meteor': minor
---

Migrate chat.postMessage endpoint to make it openAPI and AJV compatible
164 changes: 153 additions & 11 deletions apps/meteor/app/api/server/v1/chat.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Message } from '@rocket.chat/core-services';
import type { IMessage, IThreadMainMessage } from '@rocket.chat/core-typings';
import type { IMessage, MessageAttachment, IThreadMainMessage } from '@rocket.chat/core-typings';
import { MessageTypes } from '@rocket.chat/message-types';
import { Messages, Users, Rooms, Subscriptions } from '@rocket.chat/models';
import {
Expand All @@ -11,7 +11,6 @@ import {
isChatDeleteProps,
isChatSyncMessagesProps,
isChatGetMessageProps,
isChatPostMessageProps,
isChatSearchProps,
isChatSendMessageProps,
isChatIgnoreUserProps,
Expand Down Expand Up @@ -119,6 +118,132 @@ const ChatUnfollowMessageLocalSchema = {
additionalProperties: false,
};

//chat.postMessage starts
type ChatPostMessage =
| {
roomId: string | string[];
text?: string;
alias?: string;
emoji?: string;
avatar?: string;
attachments?: MessageAttachment[];
customFields?: IMessage['customFields'];
}
| {
channel: string | string[];
text?: string;
alias?: string;
emoji?: string;
avatar?: string;
attachments?: MessageAttachment[];
customFields?: IMessage['customFields'];
};

const ChatPostMessageSchema = {
oneOf: [
{
type: 'object',
properties: {
roomId: {
oneOf: [
{ type: 'string' },
{
type: 'array',
items: {
type: 'string',
},
},
],
},
text: {
type: 'string',
nullable: true,
},
alias: {
type: 'string',
nullable: true,
},
emoji: {
type: 'string',
nullable: true,
},
avatar: {
type: 'string',
nullable: true,
},
attachments: {
type: 'array',
items: {
type: 'object',
},
nullable: true,
},
tmid: {
type: 'string',
},
customFields: {
type: 'object',
nullable: true,
},
parseUrls: {
type: 'boolean',
},
},
required: ['roomId'],
additionalProperties: false,
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Mar 13, 2026

Choose a reason for hiding this comment

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

P1: chat.postMessage schema rejects tmid when using channel, breaking thread replies for channel-based requests.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/app/api/server/v1/chat.ts, line 193:

<comment>`chat.postMessage` schema rejects `tmid` when using `channel`, breaking thread replies for channel-based requests.</comment>

<file context>
@@ -119,6 +118,132 @@ const ChatUnfollowMessageLocalSchema = {
+				},
+			},
+			required: ['roomId'],
+			additionalProperties: false,
+		},
+		{
</file context>
Fix with Cubic

},
{
type: 'object',
properties: {
channel: {
oneOf: [
{ type: 'string' },
{
type: 'array',
items: {
type: 'string',
},
},
],
},
text: {
type: 'string',
nullable: true,
},
alias: {
type: 'string',
nullable: true,
},
emoji: {
type: 'string',
nullable: true,
},
avatar: {
type: 'string',
nullable: true,
},
attachments: {
type: 'array',
items: {
type: 'object',
},
nullable: true,
},
customFields: {
type: 'object',
nullable: true,
},
parseUrls: {
type: 'boolean',
},
},
required: ['channel'],
additionalProperties: false,
Comment on lines +122 to +241
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 13, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

channel + tmid is rejected by the new validator (behavior regression risk).

Line 181 allows tmid in the roomId variant, but Lines 196-241 omit tmid in the channel variant. With additionalProperties: false, payloads like { channel, tmid, text } fail validation. Also, the TS type omits tmid/parseUrls while the schema accepts them, so the typed contract diverges from runtime.

Proposed fix
 type ChatPostMessage =
 	| {
 			roomId: string | string[];
 			text?: string;
 			alias?: string;
 			emoji?: string;
 			avatar?: string;
 			attachments?: MessageAttachment[];
+			tmid?: string;
 			customFields?: IMessage['customFields'];
+			parseUrls?: boolean;
 	  }
 	| {
 			channel: string | string[];
 			text?: string;
 			alias?: string;
 			emoji?: string;
 			avatar?: string;
 			attachments?: MessageAttachment[];
+			tmid?: string;
 			customFields?: IMessage['customFields'];
+			parseUrls?: boolean;
 	  };
@@
 		{
 			type: 'object',
 			properties: {
 				channel: {
@@
 				attachments: {
 					type: 'array',
 					items: {
 						type: 'object',
 					},
 					nullable: true,
 				},
+				tmid: {
+					type: 'string',
+				},
 				customFields: {
 					type: 'object',
 					nullable: true,
 				},
Based on learnings: In Rocket.Chat OpenAPI migration PRs for endpoints under `apps/meteor/app/api/server/v1`, avoid introducing logic changes and preserve behavior.

Also applies to: 255-255, 689-740

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/meteor/app/api/server/v1/chat.ts` around lines 122 - 241, The validator
schema ChatPostMessageSchema currently allows tmid (and parseUrls) only in the
roomId branch and the TypeScript union ChatPostMessage omits tmid/parseUrls
entirely, causing valid payloads like { channel, tmid, text } to be rejected and
runtime/TS mismatch; update the second (channel) schema object to include the
same tmid and parseUrls properties (matching types from the roomId branch) and
set them nullable/optional as appropriate, and update the ChatPostMessage TS
union to include tmid?: string and parseUrls?: boolean on both variants so the
runtime schema and TS types preserve existing behavior.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

do we want tmid and parseURls in schema or not?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧩 Analysis chain

Let me check the original schema and what processWebhookMessage actually consumes before giving a definitive answer.


🏁 Script executed:

#!/bin/bash
# Check processWebhookMessage for tmid and parseUrls usage
rg -n "tmid|parseUrls|parseUrl" apps/meteor/app/lib/server/functions/processWebhookMessage.ts 2>/dev/null || \
  fd -e ts -g '*processWebhookMessage*' --exec rg -n "tmid|parseUrls|parseUrl" {}

# Also check the original ChatPostMessageSchema from rest-typings (it was deleted, check via git)
git show HEAD:packages/rest-typings/src/v1/chat.ts 2>/dev/null | grep -A 5 -B 2 "tmid\|parseUrls" | head -60

# Check what the deleted file had
git diff HEAD~1 HEAD -- packages/rest-typings/src/v1/chat.ts 2>/dev/null | grep -A 3 "tmid\|parseUrls" | head -60

Length of output: 1401


@CodeMatrix1Yes, both tmid and parseUrls should be in the schema (in both variants), and in the TypeScript type. Here's the concrete evidence:

processWebhookMessage actively reads both fields when building the outgoing message:

  • Line 102: parseUrls: messageObj.parseUrls !== undefined ? messageObj.parseUrls : !messageObj.attachments
  • Line 105: tmid: messageObj.tmid

And the original ChatPostMessageSchema in packages/rest-typings (which this PR deletes) included both fields in both variants (roomId and channel). The current schema already has parseUrls in both branches, but tmid is missing from the channel branch.

Since both schema objects have additionalProperties: false, any payload like { channel: "...", tmid: "...", text: "..." } will be rejected by the validator — a silent behavior regression compared to the old schema.

Recommended fix:

  1. Add tmid to the channel branch of ChatPostMessageSchema (it's already in the roomId branch):
 				attachments: {
 					type: 'array',
 					items: { type: 'object' },
 					nullable: true,
 				},
+				tmid: {
+					type: 'string',
+				},
 				customFields: {
 					type: 'object',
 					nullable: true,
 				},
  1. Add tmid and parseUrls to both variants of the ChatPostMessage TypeScript type so the type contract matches the runtime schema:
 type ChatPostMessage =
 	| {
 			roomId: string | string[];
 			text?: string;
 			alias?: string;
 			emoji?: string;
 			avatar?: string;
 			attachments?: MessageAttachment[];
+			tmid?: string;
 			customFields?: IMessage['customFields'];
+			parseUrls?: boolean;
 	  }
 	| {
 			channel: string | string[];
 			text?: string;
 			alias?: string;
 			emoji?: string;
 			avatar?: string;
 			attachments?: MessageAttachment[];
+			tmid?: string;
 			customFields?: IMessage['customFields'];
+			parseUrls?: boolean;
 	  };

🧠 Learnings used
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39425
File: apps/meteor/client/lib/chats/flows/processMessageUploads.ts:112-119
Timestamp: 2026-03-11T18:17:53.972Z
Learning: In `apps/meteor/client/lib/chats/flows/processMessageUploads.ts`, when sending multiple file uploads, each file is confirmed via its own `/rooms.mediaConfirm/${rid}/${fileId}` call and produces a separate message. Only the first file's confirm payload carries the composed message text (`msg`); all subsequent files receive `msg: ''`. This one-message-per-file behavior is intentional by design — do not flag it as a bug or suggest batching into a single message.

Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 39003
File: apps/meteor/client/lib/chats/flows/sendMessage.ts:39-45
Timestamp: 2026-02-24T19:16:35.307Z
Learning: In apps/meteor/client/lib/chats/flows/sendMessage.ts, when sdk.call('sendMessage', ...) throws an error, the message is intentionally left with temp: true (not deleted or cleaned up) to support a future retry UI feature. This allows users to retry sending failed messages rather than losing them.

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.

Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39414
File: apps/meteor/app/api/server/v1/rooms.ts:1241-1297
Timestamp: 2026-03-10T08:13:52.153Z
Learning: In the RocketChat/Rocket.Chat OpenAPI migration PRs for endpoints under apps/meteor/app/api/server/v1/rooms.ts, the pattern `ajv.compile<void>({...})` is intentionally used for the 200 response schema even when the endpoint returns `{ success: true }`. This is an established convention across all migrated endpoints (rooms.leave, rooms.favorite, rooms.delete, rooms.muteUser, rooms.unmuteUser). Do not flag this as a type mismatch during reviews of these migration PRs.

Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39230
File: apps/meteor/app/api/server/v1/chat.ts:214-222
Timestamp: 2026-03-03T11:11:48.541Z
Learning: In apps/meteor/server/lib/moderation/reportMessage.ts, the reportMessage function validates that description is not empty or whitespace-only with `if (!description.trim())`. When migrating the chat.reportMessage endpoint to OpenAPI, adding minLength validation to the schema preserves this existing behavior.

Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39340
File: apps/meteor/app/api/server/v1/im.ts:1349-1398
Timestamp: 2026-03-12T10:26:26.697Z
Learning: In `apps/meteor/app/api/server/v1/im.ts` (PR `#39340`), the `DmEndpoints` type intentionally includes temporary stub entries for `/v1/im.kick`, `/v1/dm.kick`, `/v1/im.leave`, and `/v1/dm.leave` (using `DmKickProps` and `DmLeaveProps`) even though no route handlers exist for them yet. These stubs were added to preserve type compatibility after removing the original `DmLeaveProps` and related files. They are planned for cleanup in a follow-up PR. Do not flag these as missing implementations when reviewing this file until the follow-up is merged.

Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 39545
File: apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts:59-61
Timestamp: 2026-03-11T22:04:20.529Z
Learning: In `apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts`, the `msg.u._id === uid` early-return in the `streamNewMessage` handler is intentional: the "New messages" indicator is designed to notify about messages from other users only. Self-sent messages — including those sent from a different session/device — are always skipped, by design. Do not flag this as a multi-session regression.

Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/page-objects/fragments/home-content.ts:60-82
Timestamp: 2026-02-24T19:36:55.089Z
Learning: In RocketChat/Rocket.Chat e2e tests (apps/meteor/tests/e2e/page-objects/fragments/home-content.ts), thread message preview listitems do not have aria-roledescription="message", so lastThreadMessagePreview locator cannot be scoped to messageListItems (which filters for aria-roledescription="message"). It should remain scoped to page.getByRole('listitem') or mainMessageList.getByRole('listitem').

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.

Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39425
File: apps/meteor/app/api/server/v1/rooms.ts:294-296
Timestamp: 2026-03-11T18:15:53.272Z
Learning: In Rocket.Chat's `rooms.mediaConfirm/:rid/:fileId` endpoint (apps/meteor/app/api/server/v1/rooms.ts), updating `file.name` from `bodyParams.fileName` without updating `file.path` is intentionally safe. The file path is keyed by `_id` (not by filename), so the stored path remains valid regardless of a rename. `file.name` only affects the display name in the message attachment; do not flag this as a path-divergence issue.

Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 39535
File: apps/meteor/app/apps/server/bridges/livechat.ts:249-249
Timestamp: 2026-03-11T16:46:55.955Z
Learning: In `apps/meteor/app/apps/server/bridges/livechat.ts`, `createVisitor()` intentionally does not propagate `externalIds` to `registerData`. This is by design: the method is deprecated (JSDoc: `deprecated Use createAndReturnVisitor instead. Note: This method does not support externalIds.`) to push callers toward `createAndReturnVisitor()`, which does support `externalIds`. Do not flag the missing `externalIds` field in `createVisitor()` as a bug.

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.

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

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.

},
],
};
//chat.postMessage ends

const isChatStarMessageLocalProps = ajv.compile<ChatStarMessageLocal>(ChatStarMessageLocalSchema);

const isChatUnstarMessageLocalProps = ajv.compile<ChatUnstarMessageLocal>(ChatUnstarMessageLocalSchema);
Expand All @@ -127,6 +252,8 @@ const isChatFollowMessageLocalProps = ajv.compile<ChatFollowMessageLocal>(ChatFo

const isChatUnfollowMessageLocalProps = ajv.compile<ChatUnfollowMessageLocal>(ChatUnfollowMessageLocalSchema);

const isChatPostMessageLocalProps = ajv.compile<ChatPostMessage>(ChatPostMessageSchema);

API.v1.addRoute(
'chat.delete',
{ authRequired: true, validateParams: isChatDeleteProps },
Expand Down Expand Up @@ -558,13 +685,29 @@ const chatEndpoints = API.v1

return API.v1.success();
},
);

API.v1.addRoute(
'chat.postMessage',
{ authRequired: true, validateParams: isChatPostMessageProps },
{
async post() {
)
.post(
'chat.postMessage',
{
authRequired: true,
body: isChatPostMessageLocalProps,
response: {
400: validateBadRequestErrorResponse,
401: validateUnauthorizedErrorResponse,
200: ajv.compile({
type: 'object',
properties: {
ts: { type: 'number' },
channel: { type: 'string' },
message: { $ref: '#/components/schemas/IMessage' },
success: { type: 'boolean', enum: [true] },
},
required: ['ts', 'channel', 'message', 'success'],
additionalProperties: false,
}),
},
},
async function action() {
const { text, attachments } = this.bodyParams;
const maxAllowedSize = settings.get<number>('Message_MaxAllowedSize') ?? 0;

Expand Down Expand Up @@ -594,8 +737,7 @@ API.v1.addRoute(
message,
});
},
},
);
);

API.v1.addRoute(
'chat.search',
Expand Down
133 changes: 0 additions & 133 deletions packages/rest-typings/src/v1/chat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -739,132 +739,6 @@ const ChatGetDeletedMessagesSchema = {

export const isChatGetDeletedMessagesProps = ajv.compile<ChatGetDeletedMessages>(ChatGetDeletedMessagesSchema);

type ChatPostMessage =
| {
roomId: string | string[];
text?: string;
alias?: string;
emoji?: string;
avatar?: string;
attachments?: MessageAttachment[];
customFields?: IMessage['customFields'];
}
| {
channel: string | string[];
text?: string;
alias?: string;
emoji?: string;
avatar?: string;
attachments?: MessageAttachment[];
customFields?: IMessage['customFields'];
};

const ChatPostMessageSchema = {
oneOf: [
{
type: 'object',
properties: {
roomId: {
oneOf: [
{ type: 'string' },
{
type: 'array',
items: {
type: 'string',
},
},
],
},
text: {
type: 'string',
nullable: true,
},
alias: {
type: 'string',
nullable: true,
},
emoji: {
type: 'string',
nullable: true,
},
avatar: {
type: 'string',
nullable: true,
},
attachments: {
type: 'array',
items: {
type: 'object',
},
nullable: true,
},
tmid: {
type: 'string',
},
customFields: {
type: 'object',
nullable: true,
},
parseUrls: {
type: 'boolean',
},
},
required: ['roomId'],
additionalProperties: false,
},
{
type: 'object',
properties: {
channel: {
oneOf: [
{ type: 'string' },
{
type: 'array',
items: {
type: 'string',
},
},
],
},
text: {
type: 'string',
nullable: true,
},
alias: {
type: 'string',
nullable: true,
},
emoji: {
type: 'string',
nullable: true,
},
avatar: {
type: 'string',
nullable: true,
},
attachments: {
type: 'array',
items: {
type: 'object',
},
nullable: true,
},
customFields: {
type: 'object',
nullable: true,
},
parseUrls: {
type: 'boolean',
},
},
required: ['channel'],
additionalProperties: false,
},
],
};

export const isChatPostMessageProps = ajv.compile<ChatPostMessage>(ChatPostMessageSchema);

type ChatGetURLPreview = {
roomId: IRoom['_id'];
url: string;
Expand Down Expand Up @@ -982,13 +856,6 @@ export type ChatEndpoints = {
};
};
};
'/v1/chat.postMessage': {
POST: (params: ChatPostMessage) => {
ts: number;
channel: IRoom;
message: IMessage;
};
};
'/v1/chat.syncThreadMessages': {
GET: (params: ChatSyncThreadMessages) => {
messages: {
Expand Down