diff --git a/packages/runtime/src/bots/__tests__/dingtalk-bridge.test.ts b/packages/runtime/src/bots/__tests__/dingtalk-bridge.test.ts index 5f2779e1d6..989c7a4a2d 100644 --- a/packages/runtime/src/bots/__tests__/dingtalk-bridge.test.ts +++ b/packages/runtime/src/bots/__tests__/dingtalk-bridge.test.ts @@ -37,27 +37,64 @@ describe('decideDingTalkClose (PR-BOT-DINGTALK-OPERATIONAL-0)', () => { }); }); +// Realistic identifier shapes. Both conversation kinds begin with `cid`, +// which is the whole point: any implementation that infers the route from +// DingTalk's own id format instead of the stamped prefix fails these. +const SINGLE_CONVERSATION_ID = 'cidEXAMPLEsingle0Ab1Cd2Ef3Gh4Ij5Kl6Mn7Op8='; +const GROUP_CONVERSATION_ID = 'cidEXAMPLEgroup9Zy8Xw7Vu6Ts5Rq4Po3Nm2Lk1J='; +const SENDER_ID = '$:LWCP_v1:$0Ab1Cd2Ef3Gh4Ij5Kl6Mn7Op8Qr9St0Uv'; +const SENDER_STAFF_ID = '01234567890123456789'; + describe('pickDingTalkSendRoute', () => { - it('routes group and direct targets while rejecting empty ids', () => { - assert.deepEqual(pickDingTalkSendRoute(' cidp-abc ', 'app-key-1', 'hello'), { - path: '/v1.0/robot/groupMessages/send', - body: { - robotCode: 'app-key-1', - openConversationId: 'cidp-abc', - msgKey: 'sampleText', - msgParam: '{"content":"hello"}', - }, - }); - assert.deepEqual(pickDingTalkSendRoute(' user-99 ', 'app-key-1', 'hi'), { + it('routes a stamped single chat to the 1:1 endpoint by staff id', () => { + assert.deepEqual(pickDingTalkSendRoute(` oto:${SENDER_STAFF_ID} `, 'app-key-1', 'hi'), { path: '/v1.0/robot/oToMessages/batchSend', body: { robotCode: 'app-key-1', - userIds: ['user-99'], + userIds: [SENDER_STAFF_ID], msgKey: 'sampleText', msgParam: '{"content":"hi"}', }, }); + }); + + it('routes a stamped group chat to the group endpoint', () => { + assert.deepEqual( + pickDingTalkSendRoute(` group:${GROUP_CONVERSATION_ID} `, 'app-key-1', 'hello'), + { + path: '/v1.0/robot/groupMessages/send', + body: { + robotCode: 'app-key-1', + openConversationId: GROUP_CONVERSATION_ID, + msgKey: 'sampleText', + msgParam: '{"content":"hello"}', + }, + }, + ); + }); + + // Unstamped ids are scheduled-task delivery targets persisted before the + // prefix existed, plus ids typed by hand into the scheduled task form. + // Both outcomes of the pre-stamping discriminator have to survive. + it('keeps an unprefixed conversation id on the group endpoint', () => { + const route = pickDingTalkSendRoute(GROUP_CONVERSATION_ID, 'app-key-1', 'hello'); + assert.equal(route?.path, '/v1.0/robot/groupMessages/send'); + assert.equal(route?.body.openConversationId, GROUP_CONVERSATION_ID); + }); + + it('keeps an unprefixed staff id on the 1:1 endpoint', () => { + // A bare staff id delivered successfully before stamping existed, so + // routing it to the group endpoint would break a working target. + const route = pickDingTalkSendRoute(SENDER_STAFF_ID, 'app-key-1', 'hi'); + assert.equal(route?.path, '/v1.0/robot/oToMessages/batchSend'); + assert.deepEqual(route?.body.userIds, [SENDER_STAFF_ID]); + }); + + it('rejects empty and prefix-only ids', () => { assert.equal(pickDingTalkSendRoute(' ', 'app-key-1', 'hi'), null); + assert.equal(pickDingTalkSendRoute('oto:', 'app-key-1', 'hi'), null); + assert.equal(pickDingTalkSendRoute('oto: ', 'app-key-1', 'hi'), null); + assert.equal(pickDingTalkSendRoute('group:', 'app-key-1', 'hi'), null); }); }); @@ -93,12 +130,13 @@ describe('classifyDingTalkSendResponse', () => { }); describe('dingTalkPayloadToEvent', () => { - it('maps direct and group messages with stable identity fallbacks', () => { + it('addresses a single chat by staff id, not by conversation id', () => { const event = dingTalkPayloadToEvent( { - senderId: 'user-1', + senderId: SENDER_ID, + senderStaffId: SENDER_STAFF_ID, senderNick: 'Alice', - conversationId: 'cidp-single', + conversationId: SINGLE_CONVERSATION_ID, conversationType: '1', text: { content: 'hello' }, robotCode: 'app-key-1', @@ -107,23 +145,81 @@ describe('dingTalkPayloadToEvent', () => { ); assert.ok(event); assert.equal(event!.platform, 'dingtalk'); - assert.equal(event!.userId, 'user-1'); + assert.equal(event!.userId, SENDER_ID); assert.equal(event!.userName, 'Alice'); - assert.equal(event!.chatId, 'cidp-single'); + assert.equal(event!.chatId, `oto:${SENDER_STAFF_ID}`); assert.equal(event!.isGroup, false); assert.equal(event!.text, 'hello'); - assert.equal(event!.sourceMessageId, 'cidp-single:1700000000000'); + assert.equal(event!.sourceMessageId, `${SINGLE_CONVERSATION_ID}:1700000000000`); + }); + + it('addresses a group chat by conversation id', () => { const groupEvent = dingTalkPayloadToEvent( { - senderId: 'user-2', - conversationId: 'cidp-group', + senderId: SENDER_ID, + senderStaffId: SENDER_STAFF_ID, + conversationId: GROUP_CONVERSATION_ID, conversationType: '2', text: { content: 'hi' }, }, 1, ); + assert.ok(groupEvent); assert.equal(groupEvent!.isGroup, true); - assert.equal(groupEvent!.userName, 'user-2'); + assert.equal(groupEvent!.chatId, `group:${GROUP_CONVERSATION_ID}`); + assert.equal(groupEvent!.userName, SENDER_ID); + }); + + it('round-trips a single chat from receive to the 1:1 send body', () => { + // The regression this guards: a 1:1 conversationId also starts with + // `cid`, so routing on the raw id sends direct replies to the group + // endpoint and DingTalk answers 400 resource.not.found. + const event = dingTalkPayloadToEvent( + { + senderId: SENDER_ID, + senderStaffId: SENDER_STAFF_ID, + conversationId: SINGLE_CONVERSATION_ID, + conversationType: '1', + text: { content: 'ping' }, + }, + 1, + ); + const route = pickDingTalkSendRoute(event!.chatId, 'app-key-1', 'pong'); + assert.equal(route?.path, '/v1.0/robot/oToMessages/batchSend'); + assert.deepEqual(route?.body.userIds, [SENDER_STAFF_ID]); + }); + + it('round-trips a group chat from receive to the group send body', () => { + const event = dingTalkPayloadToEvent( + { + senderId: SENDER_ID, + senderStaffId: SENDER_STAFF_ID, + conversationId: GROUP_CONVERSATION_ID, + conversationType: '2', + text: { content: 'ping' }, + }, + 1, + ); + const route = pickDingTalkSendRoute(event!.chatId, 'app-key-1', 'pong'); + assert.equal(route?.path, '/v1.0/robot/groupMessages/send'); + assert.equal(route?.body.openConversationId, GROUP_CONVERSATION_ID); + }); + + it('still delivers a single chat that carries no staff id', () => { + // Nothing to address a 1:1 reply to, but receiving must not depend on + // being able to reply. + const event = dingTalkPayloadToEvent( + { + senderId: SENDER_ID, + conversationId: SINGLE_CONVERSATION_ID, + conversationType: '1', + text: { content: 'hello' }, + }, + 1, + ); + assert.ok(event); + assert.equal(event!.isGroup, false); + assert.equal(event!.chatId, SINGLE_CONVERSATION_ID); }); it('drops payloads missing text or routing identity', () => { diff --git a/packages/runtime/src/bots/dingtalk-bridge.ts b/packages/runtime/src/bots/dingtalk-bridge.ts index ccd531308d..e2655a8ed9 100644 --- a/packages/runtime/src/bots/dingtalk-bridge.ts +++ b/packages/runtime/src/bots/dingtalk-bridge.ts @@ -56,6 +56,30 @@ const SEND_RETRY_DELAY_MAX_MS = 30_000; const DINGTALK_TOPIC_BOT_MESSAGES = '/v1.0/im/bot/messages/get'; +const DINGTALK_GROUP_SEND_PATH = '/v1.0/robot/groupMessages/send'; +const DINGTALK_SINGLE_SEND_PATH = '/v1.0/robot/oToMessages/batchSend'; + +/** + * Route markers stamped onto `chatId` when a message is received, so the + * send side can pick an endpoint without re-deriving the conversation + * kind. Same approach as `qq-bridge.ts`, and for the same reason: the + * conversation kind is known exactly at receive time (DingTalk sends + * `conversationType`) and cannot be recovered later from the id alone. + * + * Guessing from DingTalk's own id shapes does not work — 1:1 and group + * `conversationId` values both begin with `cid`, so a prefix test sends + * every direct reply to the group endpoint. + */ +const DINGTALK_GROUP_CHAT_PREFIX = 'group:'; +const DINGTALK_SINGLE_CHAT_PREFIX = 'oto:'; + +/** + * The discriminator this bridge used before chatIds carried a stamp. + * Retained only to route ids that predate stamping — see the unstamped + * branch of `pickDingTalkSendRoute`. + */ +const DINGTALK_LEGACY_GROUP_ID_PREFIX = 'cid'; + interface DingTalkConnectionOpenResponse { endpoint: string; ticket: string; @@ -70,6 +94,12 @@ interface DingTalkStreamFrame { interface DingTalkBotMessagePayload { senderId?: string; + /** + * The org-scoped staff id of the sender. This is the only identifier + * `/v1.0/robot/oToMessages/batchSend` accepts in `userIds`: `senderId` + * (a `$:LWCP_v1:$…` handle) is rejected with `staffId.notExisted`. + */ + senderStaffId?: string; senderNick?: string; conversationId?: string; conversationType?: '1' | '2'; // 1 = single chat, 2 = group @@ -127,6 +157,18 @@ export function buildDingTalkSingleSendBody( }; } +/** + * Pure helper: route a send to the right DingTalk REST endpoint based on + * the chatId prefix that `dingTalkPayloadToEvent` stamps. + * + * Unstamped ids are chatIds recorded before this bridge stamped a prefix + * — persisted scheduled-task delivery targets and ids typed by hand into + * the scheduled task form. They keep the pre-stamping discriminator, so + * both of its outcomes survive: a `cid…` conversation id still goes to + * the group endpoint, and a bare staff id still goes to the 1:1 endpoint, + * which is where it was delivering successfully. That guess was only ever + * wrong for a 1:1 *conversation* id, and the stamped path now covers it. + */ export function pickDingTalkSendRoute( chatId: string, robotCode: string, @@ -137,13 +179,31 @@ export function pickDingTalkSendRoute( } | null { const targetId = chatId.trim(); if (!targetId) return null; - const isGroup = targetId.startsWith('cid'); - return { - path: isGroup ? '/v1.0/robot/groupMessages/send' : '/v1.0/robot/oToMessages/batchSend', - body: isGroup - ? buildDingTalkGroupSendBody(targetId, robotCode, text) - : buildDingTalkSingleSendBody(targetId, robotCode, text), - }; + if (targetId.startsWith(DINGTALK_SINGLE_CHAT_PREFIX)) { + const staffId = targetId.slice(DINGTALK_SINGLE_CHAT_PREFIX.length).trim(); + if (!staffId) return null; + return { + path: DINGTALK_SINGLE_SEND_PATH, + body: buildDingTalkSingleSendBody(staffId, robotCode, text), + }; + } + if (targetId.startsWith(DINGTALK_GROUP_CHAT_PREFIX)) { + const openConversationId = targetId.slice(DINGTALK_GROUP_CHAT_PREFIX.length).trim(); + if (!openConversationId) return null; + return { + path: DINGTALK_GROUP_SEND_PATH, + body: buildDingTalkGroupSendBody(openConversationId, robotCode, text), + }; + } + return targetId.startsWith(DINGTALK_LEGACY_GROUP_ID_PREFIX) + ? { + path: DINGTALK_GROUP_SEND_PATH, + body: buildDingTalkGroupSendBody(targetId, robotCode, text), + } + : { + path: DINGTALK_SINGLE_SEND_PATH, + body: buildDingTalkSingleSendBody(targetId, robotCode, text), + }; } /** @@ -210,21 +270,38 @@ export function dingTalkPayloadToEvent( if (!payload || typeof payload !== 'object') return null; const content = payload.text?.content; if (typeof content !== 'string' || content.length === 0) return null; - const chatId = payload.conversationId; + const conversationId = payload.conversationId; const userId = payload.senderId; - if (typeof chatId !== 'string' || chatId.length === 0) return null; + if (typeof conversationId !== 'string' || conversationId.length === 0) return null; if (typeof userId !== 'string' || userId.length === 0) return null; + const isGroup = payload.conversationType === '2'; + const staffId = typeof payload.senderStaffId === 'string' ? payload.senderStaffId.trim() : ''; + // Stamp the route while the conversation kind is still known. A 1:1 + // reply must address the sender's staff id, not the conversation, so + // that is what the chatId carries. + // + // When the payload omits `senderStaffId` there is nothing a 1:1 reply + // can be addressed to. Fall back to the bare conversationId so the + // message still reaches the agent under a stable per-conversation key, + // but a reply to it will take the unstamped `cid…` branch and fail at + // the group endpoint — the same dead end as before this change, not a + // working path. + const chatId = isGroup + ? `${DINGTALK_GROUP_CHAT_PREFIX}${conversationId}` + : staffId + ? `${DINGTALK_SINGLE_CHAT_PREFIX}${staffId}` + : conversationId; return { platform: 'dingtalk', userId, userName: payload.senderNick ?? userId, chatId, - isGroup: payload.conversationType === '2', + isGroup, text: content, // DingTalk Stream callbacks do not carry the original message id; // use a synthetic key so downstream contracts that key off // `sourceMessageId` still get a unique value. - sourceMessageId: `${chatId}:${receivedAt}`, + sourceMessageId: `${conversationId}:${receivedAt}`, receivedAt, }; } @@ -374,14 +451,11 @@ export class DingTalkBotBridge extends WsBridgeBase implements SendCapable { } /** - * DingTalk REST send. We treat any chatId with a `cidp` prefix as a - * group conversation; pure-numeric or other prefixes route to the - * single-user batch API. The caller (main.ts) already knows whether - * the bot conversation is a group via the BotMessageEvent.isGroup - * flag, but the bridge's `sendMessage` only sees the chatId — so we - * make a conservative split based on the `conversationType` hint - * baked into the chatId structure: group conversation IDs start with - * `cid` per DingTalk's open platform docs. + * DingTalk REST send. `sendMessage` only sees the chatId, so the + * conversation kind travels inside it: `dingTalkPayloadToEvent` + * stamps `group:` or `oto:` at receive time, where DingTalk's + * `conversationType` is still available, and + * `pickDingTalkSendRoute` decodes it here. */ async sendMessage( chatId: string,