From f65fe959bb4f06eb0d67978eaa435c441ec7705a Mon Sep 17 00:00:00 2001 From: DrSmoothl <1787882683@qq.com> Date: Tue, 19 May 2026 23:36:12 +0800 Subject: [PATCH] =?UTF-8?q?fix(send-private):=20split=20c2c=20file=20segme?= =?UTF-8?q?nts=20out=20to=20sendC2cFileMessage=20(was=20shipping=20"[?= =?UTF-8?q?=E7=A9=BA=E6=B6=88=E6=81=AF]")?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit User report: `send_private_msg` with a `{type:'file', file_id}` segment fired `[ElemBuilder] file send via elems[] is group-only; use bridge. sendC2cFileMessage for c2c` and the message went out as "[空消息]". Root cause: c2c files live on `RichText.notOnlineFile` (parallel to the elems array, see proto/proton/message.ts), not inside `elems[]`. The element-builder explicitly drops c2c `{type:'file'}` elements with a warning. `sendPrivateMessage` was passing the file straight to `bridge.sendPrivateMessage(userId, [fileElement])` which then ran through `buildSendElems`, lost the file at the drop, and shipped a 0-element wire send — rendered as "[空消息]" by every receiver. Fix: split at the OneBot layer in `sendPrivateMessage`. Pull file elements out of the batch and dispatch each via `bridge.sendC2cFileMessage`; route the rest through the regular elems[] path. Matches NapCat's `dev/NapCatQQ/.../SendMsg.ts:404-415` where FILE / VIDEO / ARK / PTT each get their own sendMsg call and never mix with the regular elems[] batch. - File-only message: only the c2c file API is called (single roundtrip) - Mixed text + file: two sends, text first then file - File without file_id: dropped with a warn (was silently shipping empty messages) - UID resolution failure: surfaced as throw instead of empty send Also rewrote the element-builder fallback warn from "use sendC2cFileMessage for c2c" (now stale advice) to "BUG: c2c file reached element-builder — should be split out at the OneBot layer". Tests: 4 new regression cases under `tests/onebot/send-private-c2c-file.test.ts`. 646/646 pass. --- packages/core/src/bridge/element-builder.ts | 13 +- .../src/onebot/modules/message-actions.ts | 59 ++++++- .../onebot/send-private-c2c-file.test.ts | 153 ++++++++++++++++++ 3 files changed, 214 insertions(+), 11 deletions(-) create mode 100644 packages/core/tests/onebot/send-private-c2c-file.test.ts diff --git a/packages/core/src/bridge/element-builder.ts b/packages/core/src/bridge/element-builder.ts index 367f881e..e51414a2 100644 --- a/packages/core/src/bridge/element-builder.ts +++ b/packages/core/src/bridge/element-builder.ts @@ -393,13 +393,18 @@ export async function buildSendElems(elements: MessageElement[], ctx?: SendConte case 'file': // The group `TransElem(24)` shape works for group chats only. - // C2C files live on `RichText.notOnlineFile` instead of in the - // elems array, so `sendPrivateMessage` short-circuits before it - // ever lands here — see `bridge.sendPrivateMessage`. + // C2C files live on `RichText.notOnlineFile` (parallel to elems, + // see proto/proton/message.ts:notOnlineFile) and are routed + // out at the OneBot layer in `sendPrivateMessage` — split off + // from elems[] and dispatched via `bridge.sendC2cFileMessage`. + // If a c2c file segment still reaches us here, the caller + // forgot to route — log loudly and drop, otherwise the message + // ships as "[空消息]" (only-file case) or quietly loses the + // file (mixed case). if (ctx?.groupId !== undefined) { result.push(makeGroupFileElem(elem)); } else { - console.warn('[ElemBuilder] file send via elems[] is group-only; use bridge.sendC2cFileMessage for c2c'); + console.warn('[ElemBuilder] BUG: c2c {type:"file"} reached element-builder — should be split out at the OneBot layer (see modules/message-actions.ts::sendPrivateMessage)'); } break; diff --git a/packages/core/src/onebot/modules/message-actions.ts b/packages/core/src/onebot/modules/message-actions.ts index 911b1bc7..a47a2ac2 100644 --- a/packages/core/src/onebot/modules/message-actions.ts +++ b/packages/core/src/onebot/modules/message-actions.ts @@ -136,19 +136,64 @@ export async function sendPrivateMessage( }); if (elements.length === 0) throw new Error('message is empty'); - const receipt = await ref.bridge.sendPrivateMessage(userId, elements); - const messageId = hashMessageIdInt32(receipt.sequence, userId, PRIVATE_MESSAGE_EVENT); + // C2C `{type:'file'}` segments can't ride on the elems[] pipeline — + // c2c files live on `RichText.notOnlineFile`, parallel to elems + // (see `proto/proton/message.ts:notOnlineFile`). The element-builder + // explicitly drops them with a warn ("file send via elems[] is + // group-only"), so a private message that bundled a file alongside + // anything else used to either ship as "[空消息]" (only-file case, + // 0 elements after drop) or silently lose the file (mixed case). + // + // NapCat splits the same way (`dev/NapCatQQ/.../SendMsg.ts:404-415`): + // FILE / VIDEO / ARK / PTT each go in their own sendMsg call, + // never mixed with the regular elements. We only need it for file + // here — video/ARK/ptt already go through commonElem so the + // element-builder handles them inline. + const fileElements = elements.filter(e => e.type === 'file' && e.fileId); + const nonFileElements = elements.filter(e => e.type !== 'file'); + if (fileElements.length !== elements.filter(e => e.type === 'file').length) { + log.warn('[OneBot] private file segment without file_id — skipped (upload first via upload_private_file)'); + } - logSentMessage(false, userId, elements); + let lastReceipt: Awaited> | undefined; + if (nonFileElements.length > 0) { + lastReceipt = await ref.bridge.sendPrivateMessage(userId, nonFileElements); + logSentMessage(false, userId, nonFileElements); + } + if (fileElements.length > 0) { + // C2C file send needs the recipient's UID — resolve once and reuse. + const userUid = await ref.bridge.resolveUserUid(userId); + if (!userUid) { + throw new Error(`c2c file send: could not resolve uid for user ${userId}`); + } + for (const fileEl of fileElements) { + const fileMd5 = fileEl.md5Hex + ? Buffer.from(fileEl.md5Hex, 'hex') + : new Uint8Array(0); + lastReceipt = await ref.bridge.sendC2cFileMessage(userId, userUid, { + fileId: fileEl.fileId!, + // Fall back to a generic name — the QQ server resolves the + // file by fileUuid, not name, so a missing name only affects + // the chat bubble display. + fileName: fileEl.fileName ?? 'file', + fileSize: fileEl.fileSize ?? 0, + fileMd5, + fileHash: fileEl.fileHash, + }); + logSentMessage(false, userId, [fileEl]); + } + } + if (!lastReceipt) throw new Error('message is empty'); + const messageId = hashMessageIdInt32(lastReceipt.sequence, userId, PRIVATE_MESSAGE_EVENT); ref.cacheMessageMeta(messageId, { isGroup: false, targetId: userId, - sequence: receipt.sequence, + sequence: lastReceipt.sequence, eventName: PRIVATE_MESSAGE_EVENT, - clientSequence: receipt.clientSequence, - random: receipt.random, - timestamp: receipt.timestamp, + clientSequence: lastReceipt.clientSequence, + random: lastReceipt.random, + timestamp: lastReceipt.timestamp, }); return { messageId }; diff --git a/packages/core/tests/onebot/send-private-c2c-file.test.ts b/packages/core/tests/onebot/send-private-c2c-file.test.ts new file mode 100644 index 00000000..c9c80ed8 --- /dev/null +++ b/packages/core/tests/onebot/send-private-c2c-file.test.ts @@ -0,0 +1,153 @@ +// Regression coverage for the c2c-file-in-send_private_msg bug. +// +// Symptom in the wild: bot called `send_private_msg` with a +// `{type:'file', file_id}` segment; the OneBot module passed it +// straight to `bridge.sendPrivateMessage(userId, [fileElement])`; +// `buildSendElems` warned ("file send via elems[] is group-only") +// and dropped it; the resulting wire send had 0 elems and the chat +// rendered "[空消息]". +// +// Fix: c2c file segments are split out at the OneBot layer in +// `sendPrivateMessage` and dispatched through `bridge.sendC2cFileMessage` +// (which writes `RichText.notOnlineFile`, the correct wire slot for +// c2c files). Matches NapCat's `SendMsg.ts:404-415` which splits +// FILE / VIDEO / ARK / PTT into their own sendMsg calls and never +// mixes them with the regular elems[] batch. + +import { describe, expect, it, vi } from 'vitest'; +import type { BridgeInterface } from '../../src/bridge/bridge-interface'; +import type { OneBotInstanceContext } from '../../src/onebot/instance-context'; +import { sendPrivateMessage } from '../../src/onebot/modules/message-actions'; + +function fakeBridge(overrides: Partial = {}): BridgeInterface { + return new Proxy(overrides as BridgeInterface, { + get(target, prop) { + if (prop in target) return (target as any)[prop]; + throw new Error(`fakeBridge: '${String(prop)}' was not stubbed for this test`); + }, + }); +} + +function makeCtx(bridge: BridgeInterface): OneBotInstanceContext { + return { + uin: '10001', + bridge, + messageStore: { findEvent: () => null } as any, + cacheMessageMeta: vi.fn(), + mediaStore: {} as any, + musicSignUrl: '', + } as unknown as OneBotInstanceContext; +} + +const goodReceipt = { + messageId: 1, sequence: 100, clientSequence: 0, random: 1, timestamp: 1700000000, +}; + +describe('send_private_msg with {type:"file"} segment', () => { + it('file-only message routes through sendC2cFileMessage (not sendPrivateMessage)', async () => { + // Pure-file case — the elems[] path would have dropped this entirely + // and shipped "[空消息]". The c2c file route must take over. + const sendPrivateMessage_bridge = vi.fn(); + const sendC2cFileMessage = vi.fn(async () => goodReceipt); + const resolveUserUid = vi.fn(async () => 'u_peer'); + const bridge = fakeBridge({ + sendPrivateMessage: sendPrivateMessage_bridge, + sendC2cFileMessage, + resolveUserUid, + } as any); + const ctx = makeCtx(bridge); + + await sendPrivateMessage(ctx, 67890, [{ + type: 'file', + data: { file_id: 'uuid-abc', name: 'doc.txt', size: 123, md5: '00112233' }, + }] as any, false); + + expect(sendC2cFileMessage).toHaveBeenCalledOnce(); + expect(sendPrivateMessage_bridge).not.toHaveBeenCalled(); + + const [uin, uid, info] = sendC2cFileMessage.mock.calls[0]!; + expect(uin).toBe(67890); + expect(uid).toBe('u_peer'); + expect(info).toMatchObject({ + fileId: 'uuid-abc', + fileName: 'doc.txt', + fileSize: 123, + }); + // md5 decoded from hex (4 bytes for '00112233') + expect(info.fileMd5).toEqual(Buffer.from('00112233', 'hex')); + }); + + it('mixed file + text splits across two sends (text first, file second)', async () => { + const sendPrivateMessage_bridge = vi.fn(async () => goodReceipt); + const sendC2cFileMessage = vi.fn(async () => goodReceipt); + const resolveUserUid = vi.fn(async () => 'u_peer'); + const bridge = fakeBridge({ + sendPrivateMessage: sendPrivateMessage_bridge, + sendC2cFileMessage, + resolveUserUid, + } as any); + const ctx = makeCtx(bridge); + + await sendPrivateMessage(ctx, 67890, [ + { type: 'text', data: { text: 'here is the file:' } }, + { type: 'file', data: { file_id: 'uuid-xyz', name: 'pkg.zip' } }, + ] as any, false); + + expect(sendPrivateMessage_bridge).toHaveBeenCalledOnce(); + expect(sendC2cFileMessage).toHaveBeenCalledOnce(); + + // Text goes via the elems[] pipeline. + const [textUin, textElements] = sendPrivateMessage_bridge.mock.calls[0]!; + expect(textUin).toBe(67890); + expect(textElements).toEqual([{ type: 'text', text: 'here is the file:' }]); + + // File goes via the dedicated c2c-file API. + const [fileUin, fileUid, fileInfo] = sendC2cFileMessage.mock.calls[0]!; + expect(fileUin).toBe(67890); + expect(fileUid).toBe('u_peer'); + expect(fileInfo).toMatchObject({ fileId: 'uuid-xyz', fileName: 'pkg.zip' }); + }); + + it('file segment without file_id is skipped (with a warn-level log)', async () => { + // OneBot11 file segments are upload-by-reference — without a + // file_id there's nothing to send. Previously these slipped + // through to the elems[] path and shipped empty messages. + const sendPrivateMessage_bridge = vi.fn(async () => goodReceipt); + const sendC2cFileMessage = vi.fn(); + const bridge = fakeBridge({ + sendPrivateMessage: sendPrivateMessage_bridge, + sendC2cFileMessage, + resolveUserUid: vi.fn(), + } as any); + const ctx = makeCtx(bridge); + + await sendPrivateMessage(ctx, 67890, [ + { type: 'text', data: { text: 'with bad file segment' } }, + { type: 'file', data: {} }, // no file_id + ] as any, false); + + // Text still went out, file dropped (no c2c file call). + expect(sendPrivateMessage_bridge).toHaveBeenCalledOnce(); + expect(sendC2cFileMessage).not.toHaveBeenCalled(); + }); + + it('throws when uid cannot be resolved for a file-only message', async () => { + // C2C file send requires the recipient's UID for the RichText. + // notOnlineFile routing. If lookup fails we can't proceed — + // surfacing the error is better than shipping a c2c send with + // an empty uid. + const sendC2cFileMessage = vi.fn(); + const bridge = fakeBridge({ + sendPrivateMessage: vi.fn(), + sendC2cFileMessage, + resolveUserUid: vi.fn(async () => ''), // intentional empty + } as any); + const ctx = makeCtx(bridge); + + await expect(sendPrivateMessage(ctx, 67890, [ + { type: 'file', data: { file_id: 'uuid-orphan', name: 'x.txt' } }, + ] as any, false)).rejects.toThrow(/could not resolve uid/); + + expect(sendC2cFileMessage).not.toHaveBeenCalled(); + }); +});