From 5c6041ea04fba8fe6c0d43a46cfa1adc0013c794 Mon Sep 17 00:00:00 2001 From: Ricardo Garim Date: Wed, 10 Dec 2025 21:14:23 -0300 Subject: [PATCH 01/15] fix: add agent locking to prevent race condition in chat assignment --- .../app/livechat/server/lib/RoutingManager.ts | 82 ++++++++++++------- packages/core-typings/src/ILivechatAgent.ts | 2 + .../model-typings/src/models/IUsersModel.ts | 3 + packages/models/src/models/Users.ts | 39 ++++++++- 4 files changed, 95 insertions(+), 31 deletions(-) diff --git a/apps/meteor/app/livechat/server/lib/RoutingManager.ts b/apps/meteor/app/livechat/server/lib/RoutingManager.ts index 56f4f6b8a3ad6..ca96a84a91167 100644 --- a/apps/meteor/app/livechat/server/lib/RoutingManager.ts +++ b/apps/meteor/app/livechat/server/lib/RoutingManager.ts @@ -257,19 +257,35 @@ export const RoutingManager: Routing = { return room; } - try { - await callbacks.run('livechat.checkAgentBeforeTakeInquiry', { - agent, - inquiry, - options, + const lockAcquired = await Users.acquireAgentLock(agent.agentId); + if (!lockAcquired) { + logger.debug({ + msg: 'Cannot take inquiry because agent is currently locked by another process', + agentId: agent.agentId, + inquiryId: _id, }); - } catch (e) { if (options.clientAction && !options.forwardingToDepartment) { - throw e; + throw new Error('error-agent-is-locked'); } agent = null; } + if (agent) { + try { + await callbacks.run('livechat.checkAgentBeforeTakeInquiry', { + agent, + inquiry, + options, + }); + } catch (e) { + await Users.releaseAgentLock(agent.agentId); + if (options.clientAction && !options.forwardingToDepartment) { + throw e; + } + agent = null; + } + } + if (!agent) { logger.debug({ msg: 'Cannot take inquiry. Precondition failed for agent', inquiryId: inquiry._id }); const cbRoom = await callbacks.run<'livechat.onAgentAssignmentFailed'>('livechat.onAgentAssignmentFailed', room, { @@ -279,35 +295,41 @@ export const RoutingManager: Routing = { return cbRoom; } - const result = await LivechatInquiry.takeInquiry(_id, inquiry.lockedAt); - if (result.modifiedCount === 0) { - logger.error({ msg: 'Failed to take inquiry, could not match lockedAt', inquiryId: _id, lockedAt: inquiry.lockedAt }); - throw new Error('error-taking-inquiry-lockedAt-mismatch'); - } + try { + const result = await LivechatInquiry.takeInquiry(_id, inquiry.lockedAt); + if (result.modifiedCount === 0) { + logger.error({ msg: 'Failed to take inquiry, could not match lockedAt', inquiryId: _id, lockedAt: inquiry.lockedAt }); + throw new Error('error-taking-inquiry-lockedAt-mismatch'); + } - logger.info({ msg: 'Inquiry taken by agent', inquiryId: inquiry._id, agentId: agent.agentId }); + logger.info({ msg: 'Inquiry taken by agent', inquiryId: inquiry._id, agentId: agent.agentId }); - // assignAgent changes the room data to add the agent serving the conversation. afterTakeInquiry expects room object to be updated - const { inquiry: returnedInquiry, user } = await this.assignAgent(inquiry as InquiryWithAgentInfo, agent); - const roomAfterUpdate = await LivechatRooms.findOneById(rid); + // assignAgent changes the room data to add the agent serving the conversation. afterTakeInquiry expects room object to be updated + const { inquiry: returnedInquiry, user } = await this.assignAgent(inquiry as InquiryWithAgentInfo, agent); + const roomAfterUpdate = await LivechatRooms.findOneById(rid); - if (!roomAfterUpdate) { - // This should never happen - throw new Error('error-room-not-found'); - } + if (!roomAfterUpdate) { + // This should never happen + throw new Error('error-room-not-found'); + } - void Apps.self?.triggerEvent(AppEvents.IPostLivechatAgentAssigned, { room: roomAfterUpdate, user }); - void afterTakeInquiry({ inquiry: returnedInquiry, room: roomAfterUpdate, agent }); + void Apps.self?.triggerEvent(AppEvents.IPostLivechatAgentAssigned, { room: roomAfterUpdate, user }); + void afterTakeInquiry({ inquiry: returnedInquiry, room: roomAfterUpdate, agent }); - void notifyOnLivechatInquiryChangedById(inquiry._id, 'updated', { - status: LivechatInquiryStatus.TAKEN, - takenAt: new Date(), - defaultAgent: undefined, - estimatedInactivityCloseTimeAt: undefined, - queuedAt: undefined, - }); + void notifyOnLivechatInquiryChangedById(inquiry._id, 'updated', { + status: LivechatInquiryStatus.TAKEN, + takenAt: new Date(), + defaultAgent: undefined, + estimatedInactivityCloseTimeAt: undefined, + queuedAt: undefined, + }); - return roomAfterUpdate; + return roomAfterUpdate; + } finally { + if (agent?.agentId) { + await Users.releaseAgentLock(agent.agentId); + } + } }, async transferRoom(room, guest, transferData) { diff --git a/packages/core-typings/src/ILivechatAgent.ts b/packages/core-typings/src/ILivechatAgent.ts index 4a67930942b2d..4bc9cb874d317 100644 --- a/packages/core-typings/src/ILivechatAgent.ts +++ b/packages/core-typings/src/ILivechatAgent.ts @@ -14,6 +14,8 @@ export interface ILivechatAgent extends IUser { lastRoutingTime: Date; livechatStatusSystemModified?: boolean; openBusinessHours?: string[]; + agentLockedAt?: Date; + agentLocked?: boolean; } export type AvailableAgentsAggregation = { diff --git a/packages/model-typings/src/models/IUsersModel.ts b/packages/model-typings/src/models/IUsersModel.ts index 2dacc04c7aa6b..20c794f681935 100644 --- a/packages/model-typings/src/models/IUsersModel.ts +++ b/packages/model-typings/src/models/IUsersModel.ts @@ -138,6 +138,9 @@ export interface IUsersModel extends IBaseModel { queueInfo: { chats: number; chatsForDepartment?: number }; }>; + acquireAgentLock(agentId: IUser['_id'], lockTimeoutMs?: number): Promise; + releaseAgentLock(agentId: IUser['_id']): Promise; + findAllResumeTokensByUserId(userId: IUser['_id']): Promise<{ tokens: IMeteorLoginToken[] }[]>; findActiveByUsernameOrNameRegexWithExceptionsAndConditions( diff --git a/packages/models/src/models/Users.ts b/packages/models/src/models/Users.ts index abb5fe6bf1447..80cc9ee46e15a 100644 --- a/packages/models/src/models/Users.ts +++ b/packages/models/src/models/Users.ts @@ -822,11 +822,48 @@ export class UsersRaw extends BaseRaw> implements IU lastAssignTime?: Date; lastRoutingTime?: Date; queueInfo: { chats: number }; - }>(aggregate) + }>(aggregate, { + readConcern: { level: 'majority' }, + readPreference: 'primary', + }) .toArray(); return agent; } + async acquireAgentLock(agentId: IUser['_id'], lockTimeoutMs = 5000): Promise { + const result = await this.updateOne( + { + _id: agentId, + $or: [{ agentLocked: { $exists: false } }, { agentLockedAt: { $lt: new Date(Date.now() - lockTimeoutMs) } }], + }, + { + $set: { + agentLocked: true, + agentLockedAt: new Date(), + }, + }, + ); + + return result.modifiedCount > 0; + } + + async releaseAgentLock(agentId: IUser['_id']): Promise { + const result = await this.updateOne( + { + _id: agentId, + agentLocked: true, + }, + { + $unset: { + agentLocked: 1, + agentLockedAt: 1, + }, + }, + ); + + return result.modifiedCount > 0; + } + findAllResumeTokensByUserId(userId: IUser['_id']): Promise<{ tokens: IMeteorLoginToken[] }[]> { return this.col .aggregate<{ tokens: IMeteorLoginToken[] }>([ From eac82795d67ccef3c4b8ed40c6e86032eeeda485 Mon Sep 17 00:00:00 2001 From: Ricardo Garim Date: Mon, 15 Dec 2025 12:27:39 -0300 Subject: [PATCH 02/15] refactor: check for non-locked users while getting next available agent --- packages/models/src/models/Users.ts | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/packages/models/src/models/Users.ts b/packages/models/src/models/Users.ts index 80cc9ee46e15a..c6fbabf3338a0 100644 --- a/packages/models/src/models/Users.ts +++ b/packages/models/src/models/Users.ts @@ -58,6 +58,15 @@ const queryStatusAgentOnline = (extraFilters = {}, isLivechatEnabledWhenAgentIdl }), }); +const queryAvailableAgentsForSelection = (extraFilters = {}, isLivechatEnabledWhenAgentIdle?: boolean): Filter => ({ + ...queryStatusAgentOnline(extraFilters, isLivechatEnabledWhenAgentIdle), + $and: [ + { + $or: [{ agentLocked: { $exists: false } }, { agentLockedAt: { $lt: new Date(Date.now() - 5000) } }], + }, + ], +}); + export class UsersRaw extends BaseRaw> implements IUsersModel { constructor(db: Db, trash?: Collection>) { super(db, 'users', trash, { @@ -586,7 +595,7 @@ export class UsersRaw extends BaseRaw> implements IU isEnabledWhenAgentIdle?: boolean, ignoreUsernames?: string[], ): Promise<{ agentId: string; username?: string; lastRoutingTime?: Date; count: number; departments?: any[] }> { - const match = queryStatusAgentOnline( + const match = queryAvailableAgentsForSelection( { ...(ignoreAgentId && { _id: { $ne: ignoreAgentId } }), ...(ignoreUsernames?.length && { username: { $nin: ignoreUsernames } }) }, isEnabledWhenAgentIdle, ); @@ -667,7 +676,7 @@ export class UsersRaw extends BaseRaw> implements IU isEnabledWhenAgentIdle?: boolean, ignoreUsernames?: string[], ): Promise<{ agentId: string; username?: string; lastRoutingTime?: Date; departments?: any[] }> { - const match = queryStatusAgentOnline( + const match = queryAvailableAgentsForSelection( { ...(ignoreAgentId && { _id: { $ne: ignoreAgentId } }), ...(ignoreUsernames?.length && { username: { $nin: ignoreUsernames } }) }, isEnabledWhenAgentIdle, ); @@ -1963,7 +1972,7 @@ export class UsersRaw extends BaseRaw> implements IU username: { $nin: unavailableAgents }, }; - const query = queryStatusAgentOnline(extraFilters, enabledWhenAgentIdle); + const query = queryAvailableAgentsForSelection(extraFilters, enabledWhenAgentIdle); const sort: Record = { livechatCount: 1, From c4ae95da000fb0717631dee7a68126b70b3950d2 Mon Sep 17 00:00:00 2001 From: Ricardo Garim Date: Wed, 10 Dec 2025 22:15:42 -0300 Subject: [PATCH 03/15] test: make inquiries creation sync --- .../omnichannel-assign-room-tags.spec.ts | 15 ++++-- .../omnichannel-chat-transfers.spec.ts | 54 +++++++++---------- ...annel-contact-center-chats-filters.spec.ts | 7 ++- ...omnichannel-contact-center-filters.spec.ts | 46 ++++++++-------- .../end-to-end/api/livechat/04-dashboards.ts | 14 ++--- 5 files changed, 70 insertions(+), 66 deletions(-) diff --git a/apps/meteor/tests/e2e/omnichannel/omnichannel-assign-room-tags.spec.ts b/apps/meteor/tests/e2e/omnichannel/omnichannel-assign-room-tags.spec.ts index 187d3428a39ee..8b31f43613868 100644 --- a/apps/meteor/tests/e2e/omnichannel/omnichannel-assign-room-tags.spec.ts +++ b/apps/meteor/tests/e2e/omnichannel/omnichannel-assign-room-tags.spec.ts @@ -52,10 +52,17 @@ test.describe('OC - Tags Visibility', () => { }); test.beforeAll('Create conversations', async ({ api }) => { - conversations = await Promise.all([ - createConversation(api, { visitorName: visitorA.name, agentId: 'user1', departmentId: departmentA.data._id }), - createConversation(api, { visitorName: visitorB.name, agentId: 'user1', departmentId: departmentB.data._id }), - ]); + const conversationA = await createConversation(api, { + visitorName: visitorA.name, + agentId: 'user1', + departmentId: departmentA.data._id, + }); + const conversationB = await createConversation(api, { + visitorName: visitorB.name, + agentId: 'user1', + departmentId: departmentB.data._id, + }); + conversations = [conversationA, conversationB]; }); test.beforeEach(async ({ page }) => { diff --git a/apps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.spec.ts b/apps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.spec.ts index 869ee0947d17e..06efb27747f37 100644 --- a/apps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.spec.ts +++ b/apps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.spec.ts @@ -59,20 +59,19 @@ test.describe('OC - Chat transfers [Monitor role]', () => { test.beforeAll(async ({ api }) => { const [departmentA] = departments.map(({ data }) => data); - conversations = await Promise.all([ - createConversation(api, { - agentId: `user1`, - departmentId: departmentA._id, - }), - createConversation(api, { - agentId: `user1`, - departmentId: departmentA._id, - }), - createConversation(api, { - agentId: `user1`, - departmentId: departmentA._id, - }), - ]); + const conversationA = await createConversation(api, { + agentId: `user1`, + departmentId: departmentA._id, + }); + const conversationB = await createConversation(api, { + agentId: `user1`, + departmentId: departmentA._id, + }); + const conversationC = await createConversation(api, { + agentId: `user1`, + departmentId: departmentA._id, + }); + conversations = [conversationA, conversationB, conversationC]; }); // Create monitors @@ -303,20 +302,19 @@ test.describe('OC - Chat transfers [Manager role]', () => { test.beforeAll(async ({ api }) => { const [departmentA] = departments.map(({ data }) => data); - conversations = await Promise.all([ - createConversation(api, { - agentId: `user1`, - departmentId: departmentA._id, - }), - createConversation(api, { - agentId: `user1`, - departmentId: departmentA._id, - }), - createConversation(api, { - agentId: `user1`, - departmentId: departmentA._id, - }), - ]); + const conversationA = await createConversation(api, { + agentId: `user1`, + departmentId: departmentA._id, + }); + const conversationB = await createConversation(api, { + agentId: `user1`, + departmentId: departmentA._id, + }); + const conversationC = await createConversation(api, { + agentId: `user1`, + departmentId: departmentA._id, + }); + conversations = [conversationA, conversationB, conversationC]; }); // Create sessions diff --git a/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats-filters.spec.ts b/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats-filters.spec.ts index 9a62b6b376fac..9dab229aa10ed 100644 --- a/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats-filters.spec.ts +++ b/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats-filters.spec.ts @@ -30,10 +30,9 @@ test.describe('OC - Contact Center - Chats', () => { }); test.beforeAll(async ({ api }) => { - conversations = await Promise.all([ - createConversation(api, { agentId: `user1`, visitorName: visitorA }), - createConversation(api, { agentId: `user1`, visitorName: visitorB }), - ]); + const conversationA = await createConversation(api, { agentId: `user1`, visitorName: visitorA }); + const conversationB = await createConversation(api, { agentId: `user1`, visitorName: visitorB }); + conversations = [conversationA, conversationB]; }); test.beforeEach(async ({ page }) => { diff --git a/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-filters.spec.ts b/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-filters.spec.ts index f84707292cb84..424d22794d4ed 100644 --- a/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-filters.spec.ts +++ b/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-filters.spec.ts @@ -95,36 +95,36 @@ test.describe('OC - Contact Center', async () => { test.beforeAll(async ({ api }) => { const [departmentA, departmentB] = departments.map(({ data }) => data); - conversations = await Promise.all([ - createConversation(api, { - visitorName: visitorA, - visitorToken: visitorA, - agentId: `user1`, - departmentId: departmentA._id, - }), - createConversation(api, { - visitorName: visitorB, - visitorToken: visitorB, - agentId: `user2`, - departmentId: departmentB._id, - }), - createConversation(api, { - visitorName: visitorC, - visitorToken: visitorC, - }), - ]); + const conversationA = await createConversation(api, { + visitorName: visitorA, + visitorToken: visitorA, + agentId: `user1`, + departmentId: departmentA._id, + }); + + const conversationB = await createConversation(api, { + visitorName: visitorB, + visitorToken: visitorB, + agentId: `user2`, + departmentId: departmentB._id, + }); + + const conversationC = await createConversation(api, { + visitorName: visitorC, + visitorToken: visitorC, + }); - const [conversationA, conversationB] = conversations.map(({ data }) => data); + conversations = [conversationA, conversationB, conversationC]; await Promise.all([ updateRoom(api, { - roomId: conversationA.room._id, - visitorId: conversationA.visitor._id, + roomId: conversationA.data.room._id, + visitorId: conversationA.data.visitor._id, tags: [tagA.data.name], }), updateRoom(api, { - roomId: conversationB.room._id, - visitorId: conversationB.visitor._id, + roomId: conversationB.data.room._id, + visitorId: conversationB.data.visitor._id, tags: [tagB.data.name], }), ]); diff --git a/apps/meteor/tests/end-to-end/api/livechat/04-dashboards.ts b/apps/meteor/tests/end-to-end/api/livechat/04-dashboards.ts index 377f914715b9a..6b6893859ba46 100644 --- a/apps/meteor/tests/end-to-end/api/livechat/04-dashboards.ts +++ b/apps/meteor/tests/end-to-end/api/livechat/04-dashboards.ts @@ -101,16 +101,16 @@ describe('LIVECHAT - dashboards', function () { agents.push(agent2); const roomCreationStart = moment(); - // start a few chats - const promises = Array.from(Array(TOTAL_ROOMS).keys()).map((i) => { + // start a few chats - sequentially to avoid agent lock contention + const chatInfo = []; + for await (const i of Array(TOTAL_ROOMS).keys()) { // 2 rooms by agent 1 if (i < 2) { - return startANewLivechatRoomAndTakeIt({ departmentId: department._id, agent: agent1.credentials }); + chatInfo.push(await startANewLivechatRoomAndTakeIt({ departmentId: department._id, agent: agent1.credentials })); + } else { + chatInfo.push(await startANewLivechatRoomAndTakeIt({ departmentId: department._id, agent: agent2.credentials })); } - return startANewLivechatRoomAndTakeIt({ departmentId: department._id, agent: agent2.credentials }); - }); - - const chatInfo = await Promise.all(promises); + } roomList = chatInfo.map((info) => info.room); From df1d9fcf4797124f5cbeb745af4032efc1904949 Mon Sep 17 00:00:00 2001 From: Ricardo Garim Date: Mon, 15 Dec 2025 16:25:13 -0300 Subject: [PATCH 04/15] add changeset --- .changeset/cozy-melons-march.md | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 .changeset/cozy-melons-march.md diff --git a/.changeset/cozy-melons-march.md b/.changeset/cozy-melons-march.md new file mode 100644 index 0000000000000..fad782960e8f7 --- /dev/null +++ b/.changeset/cozy-melons-march.md @@ -0,0 +1,8 @@ +--- +'@rocket.chat/model-typings': patch +'@rocket.chat/core-typings': patch +'@rocket.chat/models': patch +'@rocket.chat/meteor': patch +--- + +Prevents over-assignment of omnichannel agents beyond their max chats limit in microservices deployments by serializing agent assignment with explicit user-level locking. From fd823a3870ca18f8a712f2b3a7fbfcc86122824c Mon Sep 17 00:00:00 2001 From: Ricardo Garim Date: Tue, 16 Dec 2025 12:50:56 -0300 Subject: [PATCH 05/15] chore: add object logging --- apps/meteor/app/livechat/server/lib/RoutingManager.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/meteor/app/livechat/server/lib/RoutingManager.ts b/apps/meteor/app/livechat/server/lib/RoutingManager.ts index ca96a84a91167..e91220d727abc 100644 --- a/apps/meteor/app/livechat/server/lib/RoutingManager.ts +++ b/apps/meteor/app/livechat/server/lib/RoutingManager.ts @@ -298,11 +298,11 @@ export const RoutingManager: Routing = { try { const result = await LivechatInquiry.takeInquiry(_id, inquiry.lockedAt); if (result.modifiedCount === 0) { - logger.error({ msg: 'Failed to take inquiry, could not match lockedAt', inquiryId: _id, lockedAt: inquiry.lockedAt }); + logger.error({ msg: 'Failed to take inquiry because lockedAt did not match', inquiryId: _id, lockedAt: inquiry.lockedAt }); throw new Error('error-taking-inquiry-lockedAt-mismatch'); } - logger.info({ msg: 'Inquiry taken by agent', inquiryId: inquiry._id, agentId: agent.agentId }); + logger.info({ msg: 'Inquiry taken', inquiryId: _id, agentId: agent.agentId }); // assignAgent changes the room data to add the agent serving the conversation. afterTakeInquiry expects room object to be updated const { inquiry: returnedInquiry, user } = await this.assignAgent(inquiry as InquiryWithAgentInfo, agent); From 0574cbb0325954c172afd2086a7752262e0f1d9a Mon Sep 17 00:00:00 2001 From: Ricardo Garim Date: Tue, 16 Dec 2025 21:41:50 -0300 Subject: [PATCH 06/15] chore: remove typecast from assignAgent call --- apps/meteor/app/livechat/server/lib/RoutingManager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/meteor/app/livechat/server/lib/RoutingManager.ts b/apps/meteor/app/livechat/server/lib/RoutingManager.ts index e91220d727abc..dad7b9e6c0522 100644 --- a/apps/meteor/app/livechat/server/lib/RoutingManager.ts +++ b/apps/meteor/app/livechat/server/lib/RoutingManager.ts @@ -305,7 +305,7 @@ export const RoutingManager: Routing = { logger.info({ msg: 'Inquiry taken', inquiryId: _id, agentId: agent.agentId }); // assignAgent changes the room data to add the agent serving the conversation. afterTakeInquiry expects room object to be updated - const { inquiry: returnedInquiry, user } = await this.assignAgent(inquiry as InquiryWithAgentInfo, agent); + const { inquiry: returnedInquiry, user } = await this.assignAgent(inquiry, agent); const roomAfterUpdate = await LivechatRooms.findOneById(rid); if (!roomAfterUpdate) { From 70a1e35d36dff6ede4cfc4c340e5dd4c7a4d3775 Mon Sep 17 00:00:00 2001 From: Ricardo Garim Date: Sat, 20 Dec 2025 10:17:19 -0300 Subject: [PATCH 07/15] tests --- .../omnichannel-assign-room-tags.spec.ts | 54 +++++++---- .../omnichannel-chat-transfers.spec.ts | 91 +++++++++++++------ ...annel-contact-center-chats-filters.spec.ts | 10 +- .../omnichannel-contact-center-chats.spec.ts | 20 +++- ...omnichannel-contact-center-filters.spec.ts | 62 +++++++------ .../tests/e2e/utils/omnichannel/rooms.ts | 21 +++++ 6 files changed, 177 insertions(+), 81 deletions(-) diff --git a/apps/meteor/tests/e2e/omnichannel/omnichannel-assign-room-tags.spec.ts b/apps/meteor/tests/e2e/omnichannel/omnichannel-assign-room-tags.spec.ts index 8b31f43613868..72b43a5f5a5de 100644 --- a/apps/meteor/tests/e2e/omnichannel/omnichannel-assign-room-tags.spec.ts +++ b/apps/meteor/tests/e2e/omnichannel/omnichannel-assign-room-tags.spec.ts @@ -2,9 +2,10 @@ import { createFakeVisitor } from '../../mocks/data'; import { IS_EE } from '../config/constants'; import { Users } from '../fixtures/userStates'; import { HomeOmnichannel } from '../page-objects'; +import { setSettingValueById } from '../utils'; import { createAgent } from '../utils/omnichannel/agents'; import { addAgentToDepartment, createDepartment } from '../utils/omnichannel/departments'; -import { createConversation } from '../utils/omnichannel/rooms'; +import { createConversation, waitForInquiryToBeTaken } from '../utils/omnichannel/rooms'; import { createTag } from '../utils/omnichannel/tags'; import { test, expect } from '../utils/test'; @@ -26,6 +27,14 @@ test.describe('OC - Tags Visibility', () => { let globalTag: Awaited>; let sharedTag: Awaited>; + test.beforeAll('Configure queue settings', async ({ api }) => { + const responses = await Promise.all([ + setSettingValueById(api, 'Livechat_waiting_queue', true), + setSettingValueById(api, 'Omnichannel_queue_delay_timeout', 1), + ]); + responses.forEach((res) => expect(res.status()).toBe(200)); + }); + test.beforeAll('Create departments', async ({ api }) => { departmentA = await createDepartment(api, { name: 'Department A' }); departmentB = await createDepartment(api, { name: 'Department B' }); @@ -52,17 +61,22 @@ test.describe('OC - Tags Visibility', () => { }); test.beforeAll('Create conversations', async ({ api }) => { - const conversationA = await createConversation(api, { - visitorName: visitorA.name, - agentId: 'user1', - departmentId: departmentA.data._id, - }); - const conversationB = await createConversation(api, { - visitorName: visitorB.name, - agentId: 'user1', - departmentId: departmentB.data._id, - }); - conversations = [conversationA, conversationB]; + conversations = await Promise.all([ + createConversation(api, { + visitorName: visitorA.name, + agentId: 'user1', + departmentId: departmentA.data._id, + }), + createConversation(api, { + visitorName: visitorB.name, + agentId: 'user1', + departmentId: departmentB.data._id, + }), + ]); + await waitForInquiryToBeTaken( + api, + conversations.map((c) => c.data.room._id), + ); }); test.beforeEach(async ({ page }) => { @@ -70,12 +84,16 @@ test.describe('OC - Tags Visibility', () => { await page.goto('/'); }); - test.afterAll(async () => { - await Promise.all(conversations.map((conversation) => conversation.delete())); - await Promise.all([tagA, tagB, globalTag, sharedTag].map((tag) => tag.delete())); - await agent.delete(); - await departmentA.delete(); - await departmentB.delete(); + test.afterAll(async ({ api }) => { + await Promise.all([ + ...conversations.map((conversation) => conversation.delete()), + [tagA, tagB, globalTag, sharedTag].map((tag) => tag.delete()), + agent.delete(), + departmentA.delete(), + departmentB.delete(), + setSettingValueById(api, 'Livechat_waiting_queue', false), + setSettingValueById(api, 'Omnichannel_queue_delay_timeout', 5), + ]); }); test('Verify agent should see correct tags based on department association', async () => { diff --git a/apps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.spec.ts b/apps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.spec.ts index 06efb27747f37..a67c2e003b59e 100644 --- a/apps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.spec.ts +++ b/apps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.spec.ts @@ -4,11 +4,12 @@ import { IS_EE } from '../config/constants'; import { createAuxContext } from '../fixtures/createAuxContext'; import { Users } from '../fixtures/userStates'; import { HomeOmnichannel } from '../page-objects'; +import { setSettingValueById } from '../utils'; import { createAgent, makeAgentAvailable } from '../utils/omnichannel/agents'; import { addAgentToDepartment, createDepartment } from '../utils/omnichannel/departments'; import { createManager } from '../utils/omnichannel/managers'; import { createMonitor } from '../utils/omnichannel/monitors'; -import { createConversation } from '../utils/omnichannel/rooms'; +import { createConversation, waitForInquiryToBeTaken } from '../utils/omnichannel/rooms'; import { createOrUpdateUnit } from '../utils/omnichannel/units'; import { expect, test } from '../utils/test'; @@ -28,6 +29,14 @@ test.describe('OC - Chat transfers [Monitor role]', () => { let poOmnichannel: HomeOmnichannel; + test.beforeAll(async ({ api }) => { + const responses = await Promise.all([ + setSettingValueById(api, 'Livechat_waiting_queue', true), + setSettingValueById(api, 'Omnichannel_queue_delay_timeout', 1), + ]); + responses.forEach((res) => expect(res.status()).toBe(200)); + }); + // Create agents test.beforeAll(async ({ api }) => { agents = await Promise.all([createAgent(api, 'user1'), createAgent(api, 'user2'), createAgent(api, 'rocketchat.internal.admin.test')]); @@ -59,19 +68,25 @@ test.describe('OC - Chat transfers [Monitor role]', () => { test.beforeAll(async ({ api }) => { const [departmentA] = departments.map(({ data }) => data); - const conversationA = await createConversation(api, { - agentId: `user1`, - departmentId: departmentA._id, - }); - const conversationB = await createConversation(api, { - agentId: `user1`, - departmentId: departmentA._id, - }); - const conversationC = await createConversation(api, { - agentId: `user1`, - departmentId: departmentA._id, - }); - conversations = [conversationA, conversationB, conversationC]; + conversations = await Promise.all([ + createConversation(api, { + agentId: `user1`, + departmentId: departmentA._id, + }), + createConversation(api, { + agentId: `user1`, + departmentId: departmentA._id, + }), + createConversation(api, { + agentId: `user1`, + departmentId: departmentA._id, + }), + ]); + + await waitForInquiryToBeTaken( + api, + conversations.map((c) => c.data.room._id), + ); }); // Create monitors @@ -115,13 +130,15 @@ test.describe('OC - Chat transfers [Monitor role]', () => { await Promise.all(sessions.map(({ page }) => page.close())); }); - test.afterAll(async () => { + test.afterAll(async ({ api }) => { await Promise.all([ ...conversations.map((conversation) => conversation.delete()), ...monitors.map((monitor) => monitor.delete()), ...agents.map((agent) => agent.delete()), ...units.map((unit) => unit.delete()), ...departments.map((department) => department.delete()), + setSettingValueById(api, 'Livechat_waiting_queue', false), + setSettingValueById(api, 'Omnichannel_queue_delay_timeout', 5), ]); }); @@ -266,6 +283,14 @@ test.describe('OC - Chat transfers [Manager role]', () => { let poOmnichannel: HomeOmnichannel; + test.beforeAll(async ({ api }) => { + const responses = await Promise.all([ + setSettingValueById(api, 'Livechat_waiting_queue', true), + setSettingValueById(api, 'Omnichannel_queue_delay_timeout', 1), + ]); + responses.forEach((res) => expect(res.status()).toBe(200)); + }); + // Create agents test.beforeAll(async ({ api }) => { agents = await Promise.all([createAgent(api, 'user1'), createAgent(api, 'user2'), createAgent(api, 'rocketchat.internal.admin.test')]); @@ -302,19 +327,25 @@ test.describe('OC - Chat transfers [Manager role]', () => { test.beforeAll(async ({ api }) => { const [departmentA] = departments.map(({ data }) => data); - const conversationA = await createConversation(api, { - agentId: `user1`, - departmentId: departmentA._id, - }); - const conversationB = await createConversation(api, { - agentId: `user1`, - departmentId: departmentA._id, - }); - const conversationC = await createConversation(api, { - agentId: `user1`, - departmentId: departmentA._id, - }); - conversations = [conversationA, conversationB, conversationC]; + conversations = await Promise.all([ + createConversation(api, { + agentId: `user1`, + departmentId: departmentA._id, + }), + createConversation(api, { + agentId: `user1`, + departmentId: departmentA._id, + }), + createConversation(api, { + agentId: `user1`, + departmentId: departmentA._id, + }), + ]); + + await waitForInquiryToBeTaken( + api, + conversations.map((c) => c.data.room._id), + ); }); // Create sessions @@ -337,12 +368,14 @@ test.describe('OC - Chat transfers [Manager role]', () => { await Promise.all(sessions.map(({ page }) => page.close())); }); - test.afterAll(async () => { + test.afterAll(async ({ api }) => { await Promise.all([ ...conversations.map((conversation) => conversation.delete()), ...managers.map((manager) => manager.delete()), ...agents.map((agent) => agent.delete()), ...departments.map((department) => department.delete()), + setSettingValueById(api, 'Livechat_waiting_queue', false), + setSettingValueById(api, 'Omnichannel_queue_delay_timeout', 5), ]); }); diff --git a/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats-filters.spec.ts b/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats-filters.spec.ts index 9dab229aa10ed..a7a3a676cc7ec 100644 --- a/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats-filters.spec.ts +++ b/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats-filters.spec.ts @@ -4,7 +4,7 @@ import { Users } from '../fixtures/userStates'; import { OmnichannelChats } from '../page-objects/omnichannel-contact-center-chats'; import { setSettingValueById } from '../utils'; import { createAgent, makeAgentAvailable } from '../utils/omnichannel/agents'; -import { createConversation } from '../utils/omnichannel/rooms'; +import { createConversation, waitForInquiryToBeTaken } from '../utils/omnichannel/rooms'; import { test, expect } from '../utils/test'; test.use({ storageState: Users.user1.state }); @@ -30,9 +30,17 @@ test.describe('OC - Contact Center - Chats', () => { }); test.beforeAll(async ({ api }) => { + // run synchronously because this test runs in the CE environment, + // which lacks the waiting queue setting. with parallel execution, + // the agent lock would prevent the second assignment and keep the + // conversation in READY state (see RoutingManager.ts:118) const conversationA = await createConversation(api, { agentId: `user1`, visitorName: visitorA }); const conversationB = await createConversation(api, { agentId: `user1`, visitorName: visitorB }); conversations = [conversationA, conversationB]; + await waitForInquiryToBeTaken( + api, + conversations.map((c) => c.data.room._id), + ); }); test.beforeEach(async ({ page }) => { diff --git a/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats.spec.ts b/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats.spec.ts index 3e46bfc04960b..9e9c29cee1585 100644 --- a/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats.spec.ts +++ b/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats.spec.ts @@ -4,9 +4,10 @@ import type { Page } from '@playwright/test'; import { IS_EE } from '../config/constants'; import { Users } from '../fixtures/userStates'; import { HomeOmnichannel, OmnichannelChats } from '../page-objects'; +import { setSettingValueById } from '../utils'; import { createAgent, makeAgentAvailable } from '../utils/omnichannel/agents'; import { addAgentToDepartment, createDepartment } from '../utils/omnichannel/departments'; -import { createConversation, updateRoom } from '../utils/omnichannel/rooms'; +import { createConversation, updateRoom, waitForInquiryToBeTaken } from '../utils/omnichannel/rooms'; import { createTag } from '../utils/omnichannel/tags'; import { test, expect } from '../utils/test'; @@ -30,8 +31,10 @@ test.describe('OC - Contact Center Chats [Auto Selection]', async () => { // Allow manual on hold test.beforeAll(async ({ api }) => { const responses = await Promise.all([ - api.post('/settings/Livechat_allow_manual_on_hold', { value: true }), - api.post('/settings/Livechat_allow_manual_on_hold_upon_agent_engagement_only', { value: false }), + setSettingValueById(api, 'Livechat_allow_manual_on_hold', true), + setSettingValueById(api, 'Livechat_allow_manual_on_hold_upon_agent_engagement_only', false), + setSettingValueById(api, 'Livechat_waiting_queue', true), + setSettingValueById(api, 'Omnichannel_queue_delay_timeout', 1), ]); responses.forEach((res) => expect(res.status()).toBe(200)); }); @@ -93,6 +96,11 @@ test.describe('OC - Contact Center Chats [Auto Selection]', async () => { }), ]); + await waitForInquiryToBeTaken( + api, + conversations.map((c) => c.data.room._id), + ); + const [conversationA, conversationB] = conversations.map(({ data }) => data); await Promise.all([ @@ -127,8 +135,10 @@ test.describe('OC - Contact Center Chats [Auto Selection]', async () => { // Delete tags ...[tagA, tagB].map((tag) => tag.delete()), // Reset setting - api.post('/settings/Livechat_allow_manual_on_hold', { value: false }), - api.post('/settings/Livechat_allow_manual_on_hold_upon_agent_engagement_only', { value: true }), + setSettingValueById(api, 'Livechat_allow_manual_on_hold', false), + setSettingValueById(api, 'Livechat_allow_manual_on_hold_upon_agent_engagement_only', true), + setSettingValueById(api, 'Livechat_waiting_queue', false), + setSettingValueById(api, 'Omnichannel_queue_delay_timeout', 5), ]); }); diff --git a/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-filters.spec.ts b/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-filters.spec.ts index 424d22794d4ed..c7466849c2086 100644 --- a/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-filters.spec.ts +++ b/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-filters.spec.ts @@ -3,9 +3,10 @@ import { IS_EE } from '../config/constants'; import { Users } from '../fixtures/userStates'; import { OmnichannelContacts } from '../page-objects/omnichannel-contacts-list'; import { OmnichannelSection } from '../page-objects/omnichannel-section'; +import { setSettingValueById } from '../utils'; import { createAgent, makeAgentAvailable } from '../utils/omnichannel/agents'; import { addAgentToDepartment, createDepartment } from '../utils/omnichannel/departments'; -import { createConversation, updateRoom } from '../utils/omnichannel/rooms'; +import { createConversation, updateRoom, waitForInquiryToBeTaken } from '../utils/omnichannel/rooms'; import { createTag } from '../utils/omnichannel/tags'; import { createOrUpdateUnit } from '../utils/omnichannel/units'; import { test, expect } from '../utils/test'; @@ -32,11 +33,12 @@ test.describe('OC - Contact Center', async () => { let poContacts: OmnichannelContacts; let poOmniSection: OmnichannelSection; - // Allow manual on hold test.beforeAll(async ({ api }) => { const responses = await Promise.all([ - api.post('/settings/Livechat_allow_manual_on_hold', { value: true }), - api.post('/settings/Livechat_allow_manual_on_hold_upon_agent_engagement_only', { value: false }), + setSettingValueById(api, 'Livechat_allow_manual_on_hold', true), + setSettingValueById(api, 'Livechat_allow_manual_on_hold_upon_agent_engagement_only', false), + setSettingValueById(api, 'Livechat_waiting_queue', true), + setSettingValueById(api, 'Omnichannel_queue_delay_timeout', 1), ]); responses.forEach((res) => expect(res.status()).toBe(200)); }); @@ -95,36 +97,38 @@ test.describe('OC - Contact Center', async () => { test.beforeAll(async ({ api }) => { const [departmentA, departmentB] = departments.map(({ data }) => data); - const conversationA = await createConversation(api, { - visitorName: visitorA, - visitorToken: visitorA, - agentId: `user1`, - departmentId: departmentA._id, - }); - - const conversationB = await createConversation(api, { - visitorName: visitorB, - visitorToken: visitorB, - agentId: `user2`, - departmentId: departmentB._id, - }); + conversations = await Promise.all([ + createConversation(api, { + visitorName: visitorA, + visitorToken: visitorA, + agentId: `user1`, + departmentId: departmentA._id, + }), + createConversation(api, { + visitorName: visitorB, + visitorToken: visitorB, + agentId: `user2`, + departmentId: departmentB._id, + }), + createConversation(api, { + visitorName: visitorC, + visitorToken: visitorC, + }), + ]); - const conversationC = await createConversation(api, { - visitorName: visitorC, - visitorToken: visitorC, - }); + await waitForInquiryToBeTaken(api, [conversations[0].data.room._id, conversations[1].data.room._id]); - conversations = [conversationA, conversationB, conversationC]; + const [conversationA, conversationB] = conversations.map(({ data }) => data); await Promise.all([ updateRoom(api, { - roomId: conversationA.data.room._id, - visitorId: conversationA.data.visitor._id, + roomId: conversationA.room._id, + visitorId: conversationA.visitor._id, tags: [tagA.data.name], }), updateRoom(api, { - roomId: conversationB.data.room._id, - visitorId: conversationB.data.visitor._id, + roomId: conversationB.room._id, + visitorId: conversationB.visitor._id, tags: [tagB.data.name], }), ]); @@ -143,8 +147,10 @@ test.describe('OC - Contact Center', async () => { // Delete units ...units.map((unit) => unit.delete()), // Reset setting - api.post('/settings/Livechat_allow_manual_on_hold', { value: false }), - api.post('/settings/Livechat_allow_manual_on_hold_upon_agent_engagement_only', { value: true }), + setSettingValueById(api, 'Livechat_allow_manual_on_hold', false), + setSettingValueById(api, 'Livechat_allow_manual_on_hold_upon_agent_engagement_only', true), + setSettingValueById(api, 'Livechat_waiting_queue', false), + setSettingValueById(api, 'Omnichannel_queue_delay_timeout', 5), ]); }); diff --git a/apps/meteor/tests/e2e/utils/omnichannel/rooms.ts b/apps/meteor/tests/e2e/utils/omnichannel/rooms.ts index 4e5281e41dedd..5eca9ba849cab 100644 --- a/apps/meteor/tests/e2e/utils/omnichannel/rooms.ts +++ b/apps/meteor/tests/e2e/utils/omnichannel/rooms.ts @@ -121,3 +121,24 @@ export const createConversation = async ( }, }; }; + +export const waitForInquiryToBeTaken = async (api: BaseTest['api'], roomIds: string | string[], maxAttempts = 10, intervalMs = 1000) => { + const ids = Array.isArray(roomIds) ? roomIds : [roomIds]; + + const checkInquiry = async (roomId: string) => { + for await (const [index] of Array(maxAttempts).entries()) { + const inquiry = await api.get('/livechat/inquiries.getOne', { roomId }); + const inquiryData = await inquiry.json(); + if (inquiryData.inquiry?.status !== 'queued') { + return; + } + + if (index < maxAttempts - 1) { + await new Promise((resolve) => setTimeout(resolve, intervalMs)); + } + } + throw new Error(`Inquiry for room ${roomId} was not taken after ${maxAttempts * intervalMs}ms`); + }; + + await Promise.all(ids.map(checkInquiry)); +}; From 2d83df0189dfa80b3cb47b9e0635798ed5ed1d00 Mon Sep 17 00:00:00 2001 From: Ricardo Garim Date: Fri, 16 Jan 2026 14:30:07 -0300 Subject: [PATCH 08/15] fix: add ownership verification to agent lock --- apps/meteor/app/livechat/server/lib/RoutingManager.ts | 9 +++++---- packages/model-typings/src/models/IUsersModel.ts | 4 ++-- packages/models/src/models/Users.ts | 7 ++++--- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/apps/meteor/app/livechat/server/lib/RoutingManager.ts b/apps/meteor/app/livechat/server/lib/RoutingManager.ts index dad7b9e6c0522..0a3a581f13d96 100644 --- a/apps/meteor/app/livechat/server/lib/RoutingManager.ts +++ b/apps/meteor/app/livechat/server/lib/RoutingManager.ts @@ -257,7 +257,8 @@ export const RoutingManager: Routing = { return room; } - const lockAcquired = await Users.acquireAgentLock(agent.agentId); + const lockTime = new Date(); + const lockAcquired = await Users.acquireAgentLock(agent.agentId, lockTime); if (!lockAcquired) { logger.debug({ msg: 'Cannot take inquiry because agent is currently locked by another process', @@ -278,7 +279,7 @@ export const RoutingManager: Routing = { options, }); } catch (e) { - await Users.releaseAgentLock(agent.agentId); + await Users.releaseAgentLock(agent.agentId, lockTime); if (options.clientAction && !options.forwardingToDepartment) { throw e; } @@ -326,8 +327,8 @@ export const RoutingManager: Routing = { return roomAfterUpdate; } finally { - if (agent?.agentId) { - await Users.releaseAgentLock(agent.agentId); + if (agent?.agentId && lockAcquired) { + await Users.releaseAgentLock(agent.agentId, lockTime); } } }, diff --git a/packages/model-typings/src/models/IUsersModel.ts b/packages/model-typings/src/models/IUsersModel.ts index 20c794f681935..836a96bf8a9d8 100644 --- a/packages/model-typings/src/models/IUsersModel.ts +++ b/packages/model-typings/src/models/IUsersModel.ts @@ -138,8 +138,8 @@ export interface IUsersModel extends IBaseModel { queueInfo: { chats: number; chatsForDepartment?: number }; }>; - acquireAgentLock(agentId: IUser['_id'], lockTimeoutMs?: number): Promise; - releaseAgentLock(agentId: IUser['_id']): Promise; + acquireAgentLock(agentId: IUser['_id'], lockTime: Date, lockTimeoutMs?: number): Promise; + releaseAgentLock(agentId: IUser['_id'], lockTime: Date): Promise; findAllResumeTokensByUserId(userId: IUser['_id']): Promise<{ tokens: IMeteorLoginToken[] }[]>; diff --git a/packages/models/src/models/Users.ts b/packages/models/src/models/Users.ts index c6fbabf3338a0..5110d8ba99686 100644 --- a/packages/models/src/models/Users.ts +++ b/packages/models/src/models/Users.ts @@ -839,7 +839,7 @@ export class UsersRaw extends BaseRaw> implements IU return agent; } - async acquireAgentLock(agentId: IUser['_id'], lockTimeoutMs = 5000): Promise { + async acquireAgentLock(agentId: IUser['_id'], lockTime: Date, lockTimeoutMs = 5000): Promise { const result = await this.updateOne( { _id: agentId, @@ -848,7 +848,7 @@ export class UsersRaw extends BaseRaw> implements IU { $set: { agentLocked: true, - agentLockedAt: new Date(), + agentLockedAt: lockTime, }, }, ); @@ -856,11 +856,12 @@ export class UsersRaw extends BaseRaw> implements IU return result.modifiedCount > 0; } - async releaseAgentLock(agentId: IUser['_id']): Promise { + async releaseAgentLock(agentId: IUser['_id'], lockTime: Date): Promise { const result = await this.updateOne( { _id: agentId, agentLocked: true, + agentLockedAt: lockTime, }, { $unset: { From 48145e3120177cf6f947bf67add9d3bbbd5abf1d Mon Sep 17 00:00:00 2001 From: Ricardo Garim Date: Mon, 19 Jan 2026 07:47:08 -0300 Subject: [PATCH 09/15] chore: remove redundant condition from agent lock release --- apps/meteor/app/livechat/server/lib/RoutingManager.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/apps/meteor/app/livechat/server/lib/RoutingManager.ts b/apps/meteor/app/livechat/server/lib/RoutingManager.ts index 0a3a581f13d96..b171b57da8b89 100644 --- a/apps/meteor/app/livechat/server/lib/RoutingManager.ts +++ b/apps/meteor/app/livechat/server/lib/RoutingManager.ts @@ -327,9 +327,7 @@ export const RoutingManager: Routing = { return roomAfterUpdate; } finally { - if (agent?.agentId && lockAcquired) { - await Users.releaseAgentLock(agent.agentId, lockTime); - } + await Users.releaseAgentLock(agent.agentId, lockTime); } }, From 8908e391defe61e94f9ab63a6f85da4b6a666a4f Mon Sep 17 00:00:00 2001 From: Kevin Aleman Date: Tue, 20 Jan 2026 08:59:39 -0600 Subject: [PATCH 10/15] change locking style --- .../app/livechat/server/lib/RoutingManager.ts | 49 +++++++++++++++++-- .../omnichannel-assign-room-tags.spec.ts | 12 +---- .../omnichannel-chat-transfers.spec.ts | 24 +-------- .../omnichannel-contact-center-chats.spec.ts | 4 -- ...omnichannel-contact-center-filters.spec.ts | 4 -- 5 files changed, 47 insertions(+), 46 deletions(-) diff --git a/apps/meteor/app/livechat/server/lib/RoutingManager.ts b/apps/meteor/app/livechat/server/lib/RoutingManager.ts index b171b57da8b89..03977ecf26661 100644 --- a/apps/meteor/app/livechat/server/lib/RoutingManager.ts +++ b/apps/meteor/app/livechat/server/lib/RoutingManager.ts @@ -68,6 +68,14 @@ type Routing = { removeAllRoomSubscriptions(room: Pick, ignoreUser?: { _id: string }): Promise; assignAgent(inquiry: InquiryWithAgentInfo, agent: SelectedAgent): Promise<{ inquiry: InquiryWithAgentInfo; user: IUser }>; + conditionalLockAgent( + agentId: string, + lockTime: Date, + ): Promise<{ + acquired: boolean; + required: boolean; + unlock: () => Promise; + }>; }; export const RoutingManager: Routing = { @@ -226,6 +234,38 @@ export const RoutingManager: Routing = { return true; }, + async conditionalLockAgent( + agentId: string, + lockTime: Date, + ): Promise<{ + acquired: boolean; + required: boolean; + unlock: () => Promise; + }> { + // chat limits is only required when waiting queue is enabled + const shouldLock = settings.get('Livechat_waiting_queue'); + + if (!shouldLock) { + return { + acquired: false, + required: false, + unlock: async () => { + // no-op + }, + }; + } + + const lockAcquired = await Users.acquireAgentLock(agentId, lockTime); + + return { + acquired: !!lockAcquired, + required: true, + unlock: async () => { + await Users.releaseAgentLock(agentId, lockTime); + }, + }; + }, + async takeInquiry(inquiry, agent, options = { clientAction: false }, room) { check( agent, @@ -257,9 +297,8 @@ export const RoutingManager: Routing = { return room; } - const lockTime = new Date(); - const lockAcquired = await Users.acquireAgentLock(agent.agentId, lockTime); - if (!lockAcquired) { + const lock = await this.conditionalLockAgent(agent.agentId, new Date()); + if (!lock.acquired && lock.required) { logger.debug({ msg: 'Cannot take inquiry because agent is currently locked by another process', agentId: agent.agentId, @@ -279,7 +318,7 @@ export const RoutingManager: Routing = { options, }); } catch (e) { - await Users.releaseAgentLock(agent.agentId, lockTime); + await lock.unlock(); if (options.clientAction && !options.forwardingToDepartment) { throw e; } @@ -327,7 +366,7 @@ export const RoutingManager: Routing = { return roomAfterUpdate; } finally { - await Users.releaseAgentLock(agent.agentId, lockTime); + await lock.unlock(); } }, diff --git a/apps/meteor/tests/e2e/omnichannel/omnichannel-assign-room-tags.spec.ts b/apps/meteor/tests/e2e/omnichannel/omnichannel-assign-room-tags.spec.ts index 72b43a5f5a5de..1d59289b6bb1b 100644 --- a/apps/meteor/tests/e2e/omnichannel/omnichannel-assign-room-tags.spec.ts +++ b/apps/meteor/tests/e2e/omnichannel/omnichannel-assign-room-tags.spec.ts @@ -27,14 +27,6 @@ test.describe('OC - Tags Visibility', () => { let globalTag: Awaited>; let sharedTag: Awaited>; - test.beforeAll('Configure queue settings', async ({ api }) => { - const responses = await Promise.all([ - setSettingValueById(api, 'Livechat_waiting_queue', true), - setSettingValueById(api, 'Omnichannel_queue_delay_timeout', 1), - ]); - responses.forEach((res) => expect(res.status()).toBe(200)); - }); - test.beforeAll('Create departments', async ({ api }) => { departmentA = await createDepartment(api, { name: 'Department A' }); departmentB = await createDepartment(api, { name: 'Department B' }); @@ -84,15 +76,13 @@ test.describe('OC - Tags Visibility', () => { await page.goto('/'); }); - test.afterAll(async ({ api }) => { + test.afterAll(async () => { await Promise.all([ ...conversations.map((conversation) => conversation.delete()), [tagA, tagB, globalTag, sharedTag].map((tag) => tag.delete()), agent.delete(), departmentA.delete(), departmentB.delete(), - setSettingValueById(api, 'Livechat_waiting_queue', false), - setSettingValueById(api, 'Omnichannel_queue_delay_timeout', 5), ]); }); diff --git a/apps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.spec.ts b/apps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.spec.ts index a67c2e003b59e..ab42dc1b3a1fc 100644 --- a/apps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.spec.ts +++ b/apps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.spec.ts @@ -29,14 +29,6 @@ test.describe('OC - Chat transfers [Monitor role]', () => { let poOmnichannel: HomeOmnichannel; - test.beforeAll(async ({ api }) => { - const responses = await Promise.all([ - setSettingValueById(api, 'Livechat_waiting_queue', true), - setSettingValueById(api, 'Omnichannel_queue_delay_timeout', 1), - ]); - responses.forEach((res) => expect(res.status()).toBe(200)); - }); - // Create agents test.beforeAll(async ({ api }) => { agents = await Promise.all([createAgent(api, 'user1'), createAgent(api, 'user2'), createAgent(api, 'rocketchat.internal.admin.test')]); @@ -130,15 +122,13 @@ test.describe('OC - Chat transfers [Monitor role]', () => { await Promise.all(sessions.map(({ page }) => page.close())); }); - test.afterAll(async ({ api }) => { + test.afterAll(async () => { await Promise.all([ ...conversations.map((conversation) => conversation.delete()), ...monitors.map((monitor) => monitor.delete()), ...agents.map((agent) => agent.delete()), ...units.map((unit) => unit.delete()), ...departments.map((department) => department.delete()), - setSettingValueById(api, 'Livechat_waiting_queue', false), - setSettingValueById(api, 'Omnichannel_queue_delay_timeout', 5), ]); }); @@ -283,14 +273,6 @@ test.describe('OC - Chat transfers [Manager role]', () => { let poOmnichannel: HomeOmnichannel; - test.beforeAll(async ({ api }) => { - const responses = await Promise.all([ - setSettingValueById(api, 'Livechat_waiting_queue', true), - setSettingValueById(api, 'Omnichannel_queue_delay_timeout', 1), - ]); - responses.forEach((res) => expect(res.status()).toBe(200)); - }); - // Create agents test.beforeAll(async ({ api }) => { agents = await Promise.all([createAgent(api, 'user1'), createAgent(api, 'user2'), createAgent(api, 'rocketchat.internal.admin.test')]); @@ -368,14 +350,12 @@ test.describe('OC - Chat transfers [Manager role]', () => { await Promise.all(sessions.map(({ page }) => page.close())); }); - test.afterAll(async ({ api }) => { + test.afterAll(async () => { await Promise.all([ ...conversations.map((conversation) => conversation.delete()), ...managers.map((manager) => manager.delete()), ...agents.map((agent) => agent.delete()), ...departments.map((department) => department.delete()), - setSettingValueById(api, 'Livechat_waiting_queue', false), - setSettingValueById(api, 'Omnichannel_queue_delay_timeout', 5), ]); }); diff --git a/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats.spec.ts b/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats.spec.ts index 9e9c29cee1585..8e0368eefac70 100644 --- a/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats.spec.ts +++ b/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats.spec.ts @@ -33,8 +33,6 @@ test.describe('OC - Contact Center Chats [Auto Selection]', async () => { const responses = await Promise.all([ setSettingValueById(api, 'Livechat_allow_manual_on_hold', true), setSettingValueById(api, 'Livechat_allow_manual_on_hold_upon_agent_engagement_only', false), - setSettingValueById(api, 'Livechat_waiting_queue', true), - setSettingValueById(api, 'Omnichannel_queue_delay_timeout', 1), ]); responses.forEach((res) => expect(res.status()).toBe(200)); }); @@ -137,8 +135,6 @@ test.describe('OC - Contact Center Chats [Auto Selection]', async () => { // Reset setting setSettingValueById(api, 'Livechat_allow_manual_on_hold', false), setSettingValueById(api, 'Livechat_allow_manual_on_hold_upon_agent_engagement_only', true), - setSettingValueById(api, 'Livechat_waiting_queue', false), - setSettingValueById(api, 'Omnichannel_queue_delay_timeout', 5), ]); }); diff --git a/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-filters.spec.ts b/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-filters.spec.ts index c7466849c2086..600aa715b5816 100644 --- a/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-filters.spec.ts +++ b/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-filters.spec.ts @@ -37,8 +37,6 @@ test.describe('OC - Contact Center', async () => { const responses = await Promise.all([ setSettingValueById(api, 'Livechat_allow_manual_on_hold', true), setSettingValueById(api, 'Livechat_allow_manual_on_hold_upon_agent_engagement_only', false), - setSettingValueById(api, 'Livechat_waiting_queue', true), - setSettingValueById(api, 'Omnichannel_queue_delay_timeout', 1), ]); responses.forEach((res) => expect(res.status()).toBe(200)); }); @@ -149,8 +147,6 @@ test.describe('OC - Contact Center', async () => { // Reset setting setSettingValueById(api, 'Livechat_allow_manual_on_hold', false), setSettingValueById(api, 'Livechat_allow_manual_on_hold_upon_agent_engagement_only', true), - setSettingValueById(api, 'Livechat_waiting_queue', false), - setSettingValueById(api, 'Omnichannel_queue_delay_timeout', 5), ]); }); From c517c2235c5326fe6bfd39c4fde4e2b7fe2b15d9 Mon Sep 17 00:00:00 2001 From: Ricardo Garim Date: Tue, 20 Jan 2026 13:39:45 -0300 Subject: [PATCH 11/15] test: revert tests --- .../omnichannel-assign-room-tags.spec.ts | 31 +++++-------------- .../omnichannel-chat-transfers.spec.ts | 13 +------- ...annel-contact-center-chats-filters.spec.ts | 17 +++------- .../omnichannel-contact-center-chats.spec.ts | 16 +++------- ...omnichannel-contact-center-filters.spec.ts | 14 ++++----- .../tests/e2e/utils/omnichannel/rooms.ts | 21 ------------- .../end-to-end/api/livechat/04-dashboards.ts | 14 ++++----- 7 files changed, 32 insertions(+), 94 deletions(-) diff --git a/apps/meteor/tests/e2e/omnichannel/omnichannel-assign-room-tags.spec.ts b/apps/meteor/tests/e2e/omnichannel/omnichannel-assign-room-tags.spec.ts index 1d59289b6bb1b..187d3428a39ee 100644 --- a/apps/meteor/tests/e2e/omnichannel/omnichannel-assign-room-tags.spec.ts +++ b/apps/meteor/tests/e2e/omnichannel/omnichannel-assign-room-tags.spec.ts @@ -2,10 +2,9 @@ import { createFakeVisitor } from '../../mocks/data'; import { IS_EE } from '../config/constants'; import { Users } from '../fixtures/userStates'; import { HomeOmnichannel } from '../page-objects'; -import { setSettingValueById } from '../utils'; import { createAgent } from '../utils/omnichannel/agents'; import { addAgentToDepartment, createDepartment } from '../utils/omnichannel/departments'; -import { createConversation, waitForInquiryToBeTaken } from '../utils/omnichannel/rooms'; +import { createConversation } from '../utils/omnichannel/rooms'; import { createTag } from '../utils/omnichannel/tags'; import { test, expect } from '../utils/test'; @@ -54,21 +53,9 @@ test.describe('OC - Tags Visibility', () => { test.beforeAll('Create conversations', async ({ api }) => { conversations = await Promise.all([ - createConversation(api, { - visitorName: visitorA.name, - agentId: 'user1', - departmentId: departmentA.data._id, - }), - createConversation(api, { - visitorName: visitorB.name, - agentId: 'user1', - departmentId: departmentB.data._id, - }), + createConversation(api, { visitorName: visitorA.name, agentId: 'user1', departmentId: departmentA.data._id }), + createConversation(api, { visitorName: visitorB.name, agentId: 'user1', departmentId: departmentB.data._id }), ]); - await waitForInquiryToBeTaken( - api, - conversations.map((c) => c.data.room._id), - ); }); test.beforeEach(async ({ page }) => { @@ -77,13 +64,11 @@ test.describe('OC - Tags Visibility', () => { }); test.afterAll(async () => { - await Promise.all([ - ...conversations.map((conversation) => conversation.delete()), - [tagA, tagB, globalTag, sharedTag].map((tag) => tag.delete()), - agent.delete(), - departmentA.delete(), - departmentB.delete(), - ]); + await Promise.all(conversations.map((conversation) => conversation.delete())); + await Promise.all([tagA, tagB, globalTag, sharedTag].map((tag) => tag.delete())); + await agent.delete(); + await departmentA.delete(); + await departmentB.delete(); }); test('Verify agent should see correct tags based on department association', async () => { diff --git a/apps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.spec.ts b/apps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.spec.ts index ab42dc1b3a1fc..869ee0947d17e 100644 --- a/apps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.spec.ts +++ b/apps/meteor/tests/e2e/omnichannel/omnichannel-chat-transfers.spec.ts @@ -4,12 +4,11 @@ import { IS_EE } from '../config/constants'; import { createAuxContext } from '../fixtures/createAuxContext'; import { Users } from '../fixtures/userStates'; import { HomeOmnichannel } from '../page-objects'; -import { setSettingValueById } from '../utils'; import { createAgent, makeAgentAvailable } from '../utils/omnichannel/agents'; import { addAgentToDepartment, createDepartment } from '../utils/omnichannel/departments'; import { createManager } from '../utils/omnichannel/managers'; import { createMonitor } from '../utils/omnichannel/monitors'; -import { createConversation, waitForInquiryToBeTaken } from '../utils/omnichannel/rooms'; +import { createConversation } from '../utils/omnichannel/rooms'; import { createOrUpdateUnit } from '../utils/omnichannel/units'; import { expect, test } from '../utils/test'; @@ -74,11 +73,6 @@ test.describe('OC - Chat transfers [Monitor role]', () => { departmentId: departmentA._id, }), ]); - - await waitForInquiryToBeTaken( - api, - conversations.map((c) => c.data.room._id), - ); }); // Create monitors @@ -323,11 +317,6 @@ test.describe('OC - Chat transfers [Manager role]', () => { departmentId: departmentA._id, }), ]); - - await waitForInquiryToBeTaken( - api, - conversations.map((c) => c.data.room._id), - ); }); // Create sessions diff --git a/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats-filters.spec.ts b/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats-filters.spec.ts index a7a3a676cc7ec..9a62b6b376fac 100644 --- a/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats-filters.spec.ts +++ b/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats-filters.spec.ts @@ -4,7 +4,7 @@ import { Users } from '../fixtures/userStates'; import { OmnichannelChats } from '../page-objects/omnichannel-contact-center-chats'; import { setSettingValueById } from '../utils'; import { createAgent, makeAgentAvailable } from '../utils/omnichannel/agents'; -import { createConversation, waitForInquiryToBeTaken } from '../utils/omnichannel/rooms'; +import { createConversation } from '../utils/omnichannel/rooms'; import { test, expect } from '../utils/test'; test.use({ storageState: Users.user1.state }); @@ -30,17 +30,10 @@ test.describe('OC - Contact Center - Chats', () => { }); test.beforeAll(async ({ api }) => { - // run synchronously because this test runs in the CE environment, - // which lacks the waiting queue setting. with parallel execution, - // the agent lock would prevent the second assignment and keep the - // conversation in READY state (see RoutingManager.ts:118) - const conversationA = await createConversation(api, { agentId: `user1`, visitorName: visitorA }); - const conversationB = await createConversation(api, { agentId: `user1`, visitorName: visitorB }); - conversations = [conversationA, conversationB]; - await waitForInquiryToBeTaken( - api, - conversations.map((c) => c.data.room._id), - ); + conversations = await Promise.all([ + createConversation(api, { agentId: `user1`, visitorName: visitorA }), + createConversation(api, { agentId: `user1`, visitorName: visitorB }), + ]); }); test.beforeEach(async ({ page }) => { diff --git a/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats.spec.ts b/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats.spec.ts index 8e0368eefac70..3e46bfc04960b 100644 --- a/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats.spec.ts +++ b/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-chats.spec.ts @@ -4,10 +4,9 @@ import type { Page } from '@playwright/test'; import { IS_EE } from '../config/constants'; import { Users } from '../fixtures/userStates'; import { HomeOmnichannel, OmnichannelChats } from '../page-objects'; -import { setSettingValueById } from '../utils'; import { createAgent, makeAgentAvailable } from '../utils/omnichannel/agents'; import { addAgentToDepartment, createDepartment } from '../utils/omnichannel/departments'; -import { createConversation, updateRoom, waitForInquiryToBeTaken } from '../utils/omnichannel/rooms'; +import { createConversation, updateRoom } from '../utils/omnichannel/rooms'; import { createTag } from '../utils/omnichannel/tags'; import { test, expect } from '../utils/test'; @@ -31,8 +30,8 @@ test.describe('OC - Contact Center Chats [Auto Selection]', async () => { // Allow manual on hold test.beforeAll(async ({ api }) => { const responses = await Promise.all([ - setSettingValueById(api, 'Livechat_allow_manual_on_hold', true), - setSettingValueById(api, 'Livechat_allow_manual_on_hold_upon_agent_engagement_only', false), + api.post('/settings/Livechat_allow_manual_on_hold', { value: true }), + api.post('/settings/Livechat_allow_manual_on_hold_upon_agent_engagement_only', { value: false }), ]); responses.forEach((res) => expect(res.status()).toBe(200)); }); @@ -94,11 +93,6 @@ test.describe('OC - Contact Center Chats [Auto Selection]', async () => { }), ]); - await waitForInquiryToBeTaken( - api, - conversations.map((c) => c.data.room._id), - ); - const [conversationA, conversationB] = conversations.map(({ data }) => data); await Promise.all([ @@ -133,8 +127,8 @@ test.describe('OC - Contact Center Chats [Auto Selection]', async () => { // Delete tags ...[tagA, tagB].map((tag) => tag.delete()), // Reset setting - setSettingValueById(api, 'Livechat_allow_manual_on_hold', false), - setSettingValueById(api, 'Livechat_allow_manual_on_hold_upon_agent_engagement_only', true), + api.post('/settings/Livechat_allow_manual_on_hold', { value: false }), + api.post('/settings/Livechat_allow_manual_on_hold_upon_agent_engagement_only', { value: true }), ]); }); diff --git a/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-filters.spec.ts b/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-filters.spec.ts index 600aa715b5816..f84707292cb84 100644 --- a/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-filters.spec.ts +++ b/apps/meteor/tests/e2e/omnichannel/omnichannel-contact-center-filters.spec.ts @@ -3,10 +3,9 @@ import { IS_EE } from '../config/constants'; import { Users } from '../fixtures/userStates'; import { OmnichannelContacts } from '../page-objects/omnichannel-contacts-list'; import { OmnichannelSection } from '../page-objects/omnichannel-section'; -import { setSettingValueById } from '../utils'; import { createAgent, makeAgentAvailable } from '../utils/omnichannel/agents'; import { addAgentToDepartment, createDepartment } from '../utils/omnichannel/departments'; -import { createConversation, updateRoom, waitForInquiryToBeTaken } from '../utils/omnichannel/rooms'; +import { createConversation, updateRoom } from '../utils/omnichannel/rooms'; import { createTag } from '../utils/omnichannel/tags'; import { createOrUpdateUnit } from '../utils/omnichannel/units'; import { test, expect } from '../utils/test'; @@ -33,10 +32,11 @@ test.describe('OC - Contact Center', async () => { let poContacts: OmnichannelContacts; let poOmniSection: OmnichannelSection; + // Allow manual on hold test.beforeAll(async ({ api }) => { const responses = await Promise.all([ - setSettingValueById(api, 'Livechat_allow_manual_on_hold', true), - setSettingValueById(api, 'Livechat_allow_manual_on_hold_upon_agent_engagement_only', false), + api.post('/settings/Livechat_allow_manual_on_hold', { value: true }), + api.post('/settings/Livechat_allow_manual_on_hold_upon_agent_engagement_only', { value: false }), ]); responses.forEach((res) => expect(res.status()).toBe(200)); }); @@ -114,8 +114,6 @@ test.describe('OC - Contact Center', async () => { }), ]); - await waitForInquiryToBeTaken(api, [conversations[0].data.room._id, conversations[1].data.room._id]); - const [conversationA, conversationB] = conversations.map(({ data }) => data); await Promise.all([ @@ -145,8 +143,8 @@ test.describe('OC - Contact Center', async () => { // Delete units ...units.map((unit) => unit.delete()), // Reset setting - setSettingValueById(api, 'Livechat_allow_manual_on_hold', false), - setSettingValueById(api, 'Livechat_allow_manual_on_hold_upon_agent_engagement_only', true), + api.post('/settings/Livechat_allow_manual_on_hold', { value: false }), + api.post('/settings/Livechat_allow_manual_on_hold_upon_agent_engagement_only', { value: true }), ]); }); diff --git a/apps/meteor/tests/e2e/utils/omnichannel/rooms.ts b/apps/meteor/tests/e2e/utils/omnichannel/rooms.ts index 5eca9ba849cab..4e5281e41dedd 100644 --- a/apps/meteor/tests/e2e/utils/omnichannel/rooms.ts +++ b/apps/meteor/tests/e2e/utils/omnichannel/rooms.ts @@ -121,24 +121,3 @@ export const createConversation = async ( }, }; }; - -export const waitForInquiryToBeTaken = async (api: BaseTest['api'], roomIds: string | string[], maxAttempts = 10, intervalMs = 1000) => { - const ids = Array.isArray(roomIds) ? roomIds : [roomIds]; - - const checkInquiry = async (roomId: string) => { - for await (const [index] of Array(maxAttempts).entries()) { - const inquiry = await api.get('/livechat/inquiries.getOne', { roomId }); - const inquiryData = await inquiry.json(); - if (inquiryData.inquiry?.status !== 'queued') { - return; - } - - if (index < maxAttempts - 1) { - await new Promise((resolve) => setTimeout(resolve, intervalMs)); - } - } - throw new Error(`Inquiry for room ${roomId} was not taken after ${maxAttempts * intervalMs}ms`); - }; - - await Promise.all(ids.map(checkInquiry)); -}; diff --git a/apps/meteor/tests/end-to-end/api/livechat/04-dashboards.ts b/apps/meteor/tests/end-to-end/api/livechat/04-dashboards.ts index 6b6893859ba46..377f914715b9a 100644 --- a/apps/meteor/tests/end-to-end/api/livechat/04-dashboards.ts +++ b/apps/meteor/tests/end-to-end/api/livechat/04-dashboards.ts @@ -101,16 +101,16 @@ describe('LIVECHAT - dashboards', function () { agents.push(agent2); const roomCreationStart = moment(); - // start a few chats - sequentially to avoid agent lock contention - const chatInfo = []; - for await (const i of Array(TOTAL_ROOMS).keys()) { + // start a few chats + const promises = Array.from(Array(TOTAL_ROOMS).keys()).map((i) => { // 2 rooms by agent 1 if (i < 2) { - chatInfo.push(await startANewLivechatRoomAndTakeIt({ departmentId: department._id, agent: agent1.credentials })); - } else { - chatInfo.push(await startANewLivechatRoomAndTakeIt({ departmentId: department._id, agent: agent2.credentials })); + return startANewLivechatRoomAndTakeIt({ departmentId: department._id, agent: agent1.credentials }); } - } + return startANewLivechatRoomAndTakeIt({ departmentId: department._id, agent: agent2.credentials }); + }); + + const chatInfo = await Promise.all(promises); roomList = chatInfo.map((info) => info.room); From 9b215419c9c88f5934a1d531d1c005f5ff5a0fca Mon Sep 17 00:00:00 2001 From: Ricardo Garim Date: Tue, 20 Jan 2026 18:55:28 -0300 Subject: [PATCH 12/15] refactor: extract conditionalLockAgent to separate file with tests --- .../app/livechat/server/lib/RoutingManager.ts | 43 +------- .../server/lib/conditionalLockAgent.ts | 34 +++++++ .../server/lib/conditionalLockAgent.spec.ts | 97 +++++++++++++++++++ 3 files changed, 133 insertions(+), 41 deletions(-) create mode 100644 apps/meteor/app/livechat/server/lib/conditionalLockAgent.ts create mode 100644 apps/meteor/tests/unit/app/livechat/server/lib/conditionalLockAgent.spec.ts diff --git a/apps/meteor/app/livechat/server/lib/RoutingManager.ts b/apps/meteor/app/livechat/server/lib/RoutingManager.ts index 03977ecf26661..4ed411d57368e 100644 --- a/apps/meteor/app/livechat/server/lib/RoutingManager.ts +++ b/apps/meteor/app/livechat/server/lib/RoutingManager.ts @@ -28,6 +28,7 @@ import { updateChatDepartment, allowAgentSkipQueue, } from './Helper'; +import { conditionalLockAgent } from './conditionalLockAgent'; import { afterTakeInquiry, beforeDelegateAgent } from './hooks'; import { callbacks } from '../../../../server/lib/callbacks'; import { notifyOnLivechatInquiryChangedById, notifyOnLivechatInquiryChanged } from '../../../lib/server/lib/notifyListener'; @@ -68,14 +69,6 @@ type Routing = { removeAllRoomSubscriptions(room: Pick, ignoreUser?: { _id: string }): Promise; assignAgent(inquiry: InquiryWithAgentInfo, agent: SelectedAgent): Promise<{ inquiry: InquiryWithAgentInfo; user: IUser }>; - conditionalLockAgent( - agentId: string, - lockTime: Date, - ): Promise<{ - acquired: boolean; - required: boolean; - unlock: () => Promise; - }>; }; export const RoutingManager: Routing = { @@ -234,38 +227,6 @@ export const RoutingManager: Routing = { return true; }, - async conditionalLockAgent( - agentId: string, - lockTime: Date, - ): Promise<{ - acquired: boolean; - required: boolean; - unlock: () => Promise; - }> { - // chat limits is only required when waiting queue is enabled - const shouldLock = settings.get('Livechat_waiting_queue'); - - if (!shouldLock) { - return { - acquired: false, - required: false, - unlock: async () => { - // no-op - }, - }; - } - - const lockAcquired = await Users.acquireAgentLock(agentId, lockTime); - - return { - acquired: !!lockAcquired, - required: true, - unlock: async () => { - await Users.releaseAgentLock(agentId, lockTime); - }, - }; - }, - async takeInquiry(inquiry, agent, options = { clientAction: false }, room) { check( agent, @@ -297,7 +258,7 @@ export const RoutingManager: Routing = { return room; } - const lock = await this.conditionalLockAgent(agent.agentId, new Date()); + const lock = await conditionalLockAgent(agent.agentId, new Date()); if (!lock.acquired && lock.required) { logger.debug({ msg: 'Cannot take inquiry because agent is currently locked by another process', diff --git a/apps/meteor/app/livechat/server/lib/conditionalLockAgent.ts b/apps/meteor/app/livechat/server/lib/conditionalLockAgent.ts new file mode 100644 index 0000000000000..e4cc56d87f43e --- /dev/null +++ b/apps/meteor/app/livechat/server/lib/conditionalLockAgent.ts @@ -0,0 +1,34 @@ +import { Users } from '@rocket.chat/models'; + +import { settings } from '../../../settings/server'; + +type LockResult = { + acquired: boolean; + required: boolean; + unlock: () => Promise; +}; + +export async function conditionalLockAgent(agentId: string, lockTime: Date): Promise { + // Lock and chats limits enforcement are only required when waiting_queue is enabled + const shouldLock = settings.get('Livechat_waiting_queue'); + + if (!shouldLock) { + return { + acquired: false, + required: false, + unlock: async () => { + // no-op + }, + }; + } + + const lockAcquired = await Users.acquireAgentLock(agentId, lockTime); + + return { + acquired: !!lockAcquired, + required: true, + unlock: async () => { + await Users.releaseAgentLock(agentId, lockTime); + }, + }; +} diff --git a/apps/meteor/tests/unit/app/livechat/server/lib/conditionalLockAgent.spec.ts b/apps/meteor/tests/unit/app/livechat/server/lib/conditionalLockAgent.spec.ts new file mode 100644 index 0000000000000..74620fe9d136d --- /dev/null +++ b/apps/meteor/tests/unit/app/livechat/server/lib/conditionalLockAgent.spec.ts @@ -0,0 +1,97 @@ +import { expect } from 'chai'; +import proxyquire from 'proxyquire'; +import sinon from 'sinon'; + +const mockUsers = { + acquireAgentLock: sinon.stub(), + releaseAgentLock: sinon.stub(), +}; + +const mockSettings = { + get: sinon.stub(), +}; + +const { conditionalLockAgent } = proxyquire.noCallThru().load('../../../../../../app/livechat/server/lib/conditionalLockAgent', { + '@rocket.chat/models': { Users: mockUsers }, + '../../../settings/server': { settings: mockSettings }, +}); + +describe('conditionalLockAgent', () => { + beforeEach(() => { + mockUsers.acquireAgentLock.reset(); + mockUsers.releaseAgentLock.reset(); + mockSettings.get.reset(); + }); + + describe('when waiting_queue is enabled', () => { + beforeEach(() => { + mockSettings.get.withArgs('Livechat_waiting_queue').returns(true); + }); + + it('should return acquired: true when lock is successfully acquired', async () => { + mockUsers.acquireAgentLock.resolves(true); + + const result = await conditionalLockAgent('agent1', new Date()); + + expect(result.acquired).to.equal(true); + expect(result.required).to.equal(true); + expect(mockUsers.acquireAgentLock.calledOnce).to.equal(true); + }); + + it('should return acquired: false when lock is already held by another process', async () => { + mockUsers.acquireAgentLock.resolves(false); + + const result = await conditionalLockAgent('agent1', new Date()); + + expect(result.acquired).to.equal(false); + expect(result.required).to.equal(true); + expect(mockUsers.acquireAgentLock.calledOnce).to.equal(true); + }); + + it('should call releaseAgentLock when unlock is called', async () => { + mockUsers.acquireAgentLock.resolves(true); + mockUsers.releaseAgentLock.resolves(true); + + const lockTime = new Date(); + const result = await conditionalLockAgent('agent1', lockTime); + await result.unlock(); + + expect(mockUsers.releaseAgentLock.calledOnceWith('agent1', lockTime)).to.equal(true); + }); + + it('should simulate concurrent lock attempts - second attempt blocked', async () => { + // first call: lock acquired + mockUsers.acquireAgentLock.onFirstCall().resolves(true); + // second call: lock NOT acquired (held by first) + mockUsers.acquireAgentLock.onSecondCall().resolves(false); + + const resultA = await conditionalLockAgent('agent1', new Date()); + const resultB = await conditionalLockAgent('agent1', new Date()); + + expect(resultA.acquired).to.equal(true); + expect(resultB.acquired).to.equal(false); + expect(mockUsers.acquireAgentLock.calledTwice).to.equal(true); + }); + }); + + describe('when waiting_queue is disabled', () => { + beforeEach(() => { + mockSettings.get.withArgs('Livechat_waiting_queue').returns(false); + }); + + it('should return acquired: false and required: false without calling acquireAgentLock', async () => { + const result = await conditionalLockAgent('agent1', new Date()); + + expect(result.acquired).to.equal(false); + expect(result.required).to.equal(false); + expect(mockUsers.acquireAgentLock.called).to.equal(false); + }); + + it('should have a no-op unlock function', async () => { + const result = await conditionalLockAgent('agent1', new Date()); + await result.unlock(); // should not throw an error + + expect(mockUsers.releaseAgentLock.called).to.equal(false); + }); + }); +}); From 237826b0f7863f454dade1f0972b23bf9fbc4239 Mon Sep 17 00:00:00 2001 From: Ricardo Garim Date: Wed, 21 Jan 2026 18:59:11 -0300 Subject: [PATCH 13/15] refactor: make lockTime a local variable in conditionalLockAgent --- .../app/livechat/server/lib/RoutingManager.ts | 2 +- .../server/lib/conditionalLockAgent.ts | 3 ++- .../server/lib/conditionalLockAgent.spec.ts | 19 ++++++++++--------- 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/apps/meteor/app/livechat/server/lib/RoutingManager.ts b/apps/meteor/app/livechat/server/lib/RoutingManager.ts index 4ed411d57368e..8c6fb51c08b2d 100644 --- a/apps/meteor/app/livechat/server/lib/RoutingManager.ts +++ b/apps/meteor/app/livechat/server/lib/RoutingManager.ts @@ -258,7 +258,7 @@ export const RoutingManager: Routing = { return room; } - const lock = await conditionalLockAgent(agent.agentId, new Date()); + const lock = await conditionalLockAgent(agent.agentId); if (!lock.acquired && lock.required) { logger.debug({ msg: 'Cannot take inquiry because agent is currently locked by another process', diff --git a/apps/meteor/app/livechat/server/lib/conditionalLockAgent.ts b/apps/meteor/app/livechat/server/lib/conditionalLockAgent.ts index e4cc56d87f43e..44f4d416c18a1 100644 --- a/apps/meteor/app/livechat/server/lib/conditionalLockAgent.ts +++ b/apps/meteor/app/livechat/server/lib/conditionalLockAgent.ts @@ -8,7 +8,7 @@ type LockResult = { unlock: () => Promise; }; -export async function conditionalLockAgent(agentId: string, lockTime: Date): Promise { +export async function conditionalLockAgent(agentId: string): Promise { // Lock and chats limits enforcement are only required when waiting_queue is enabled const shouldLock = settings.get('Livechat_waiting_queue'); @@ -22,6 +22,7 @@ export async function conditionalLockAgent(agentId: string, lockTime: Date): Pro }; } + const lockTime = new Date(); const lockAcquired = await Users.acquireAgentLock(agentId, lockTime); return { diff --git a/apps/meteor/tests/unit/app/livechat/server/lib/conditionalLockAgent.spec.ts b/apps/meteor/tests/unit/app/livechat/server/lib/conditionalLockAgent.spec.ts index 74620fe9d136d..7955b231aa43c 100644 --- a/apps/meteor/tests/unit/app/livechat/server/lib/conditionalLockAgent.spec.ts +++ b/apps/meteor/tests/unit/app/livechat/server/lib/conditionalLockAgent.spec.ts @@ -31,7 +31,7 @@ describe('conditionalLockAgent', () => { it('should return acquired: true when lock is successfully acquired', async () => { mockUsers.acquireAgentLock.resolves(true); - const result = await conditionalLockAgent('agent1', new Date()); + const result = await conditionalLockAgent('agent1'); expect(result.acquired).to.equal(true); expect(result.required).to.equal(true); @@ -41,7 +41,7 @@ describe('conditionalLockAgent', () => { it('should return acquired: false when lock is already held by another process', async () => { mockUsers.acquireAgentLock.resolves(false); - const result = await conditionalLockAgent('agent1', new Date()); + const result = await conditionalLockAgent('agent1'); expect(result.acquired).to.equal(false); expect(result.required).to.equal(true); @@ -52,11 +52,12 @@ describe('conditionalLockAgent', () => { mockUsers.acquireAgentLock.resolves(true); mockUsers.releaseAgentLock.resolves(true); - const lockTime = new Date(); - const result = await conditionalLockAgent('agent1', lockTime); + const result = await conditionalLockAgent('agent1'); await result.unlock(); - expect(mockUsers.releaseAgentLock.calledOnceWith('agent1', lockTime)).to.equal(true); + expect(mockUsers.releaseAgentLock.calledOnce).to.equal(true); + expect(mockUsers.releaseAgentLock.firstCall.args[0]).to.equal('agent1'); + expect(mockUsers.releaseAgentLock.firstCall.args[1]).to.be.instanceOf(Date); }); it('should simulate concurrent lock attempts - second attempt blocked', async () => { @@ -65,8 +66,8 @@ describe('conditionalLockAgent', () => { // second call: lock NOT acquired (held by first) mockUsers.acquireAgentLock.onSecondCall().resolves(false); - const resultA = await conditionalLockAgent('agent1', new Date()); - const resultB = await conditionalLockAgent('agent1', new Date()); + const resultA = await conditionalLockAgent('agent1'); + const resultB = await conditionalLockAgent('agent1'); expect(resultA.acquired).to.equal(true); expect(resultB.acquired).to.equal(false); @@ -80,7 +81,7 @@ describe('conditionalLockAgent', () => { }); it('should return acquired: false and required: false without calling acquireAgentLock', async () => { - const result = await conditionalLockAgent('agent1', new Date()); + const result = await conditionalLockAgent('agent1'); expect(result.acquired).to.equal(false); expect(result.required).to.equal(false); @@ -88,7 +89,7 @@ describe('conditionalLockAgent', () => { }); it('should have a no-op unlock function', async () => { - const result = await conditionalLockAgent('agent1', new Date()); + const result = await conditionalLockAgent('agent1'); await result.unlock(); // should not throw an error expect(mockUsers.releaseAgentLock.called).to.equal(false); From 92fa6c93be33cfd5008750fd48592151b9ac1306 Mon Sep 17 00:00:00 2001 From: Ricardo Garim Date: Wed, 21 Jan 2026 19:16:48 -0300 Subject: [PATCH 14/15] test: remove redundant concurrent lock test from conditionalLockAgent --- .../server/lib/conditionalLockAgent.spec.ts | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/apps/meteor/tests/unit/app/livechat/server/lib/conditionalLockAgent.spec.ts b/apps/meteor/tests/unit/app/livechat/server/lib/conditionalLockAgent.spec.ts index 7955b231aa43c..50c7677db7bf4 100644 --- a/apps/meteor/tests/unit/app/livechat/server/lib/conditionalLockAgent.spec.ts +++ b/apps/meteor/tests/unit/app/livechat/server/lib/conditionalLockAgent.spec.ts @@ -59,20 +59,6 @@ describe('conditionalLockAgent', () => { expect(mockUsers.releaseAgentLock.firstCall.args[0]).to.equal('agent1'); expect(mockUsers.releaseAgentLock.firstCall.args[1]).to.be.instanceOf(Date); }); - - it('should simulate concurrent lock attempts - second attempt blocked', async () => { - // first call: lock acquired - mockUsers.acquireAgentLock.onFirstCall().resolves(true); - // second call: lock NOT acquired (held by first) - mockUsers.acquireAgentLock.onSecondCall().resolves(false); - - const resultA = await conditionalLockAgent('agent1'); - const resultB = await conditionalLockAgent('agent1'); - - expect(resultA.acquired).to.equal(true); - expect(resultB.acquired).to.equal(false); - expect(mockUsers.acquireAgentLock.calledTwice).to.equal(true); - }); }); describe('when waiting_queue is disabled', () => { From 9ebbb6af5fc2bf5961110aced01d2e0275f3bf1f Mon Sep 17 00:00:00 2001 From: Ricardo Garim Date: Thu, 22 Jan 2026 12:13:50 -0300 Subject: [PATCH 15/15] chore: remove readPreference from aggregate query --- packages/models/src/models/Users.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/models/src/models/Users.ts b/packages/models/src/models/Users.ts index 5110d8ba99686..c19417fbc5301 100644 --- a/packages/models/src/models/Users.ts +++ b/packages/models/src/models/Users.ts @@ -831,10 +831,7 @@ export class UsersRaw extends BaseRaw> implements IU lastAssignTime?: Date; lastRoutingTime?: Date; queueInfo: { chats: number }; - }>(aggregate, { - readConcern: { level: 'majority' }, - readPreference: 'primary', - }) + }>(aggregate) .toArray(); return agent; }