fix(send-private): split c2c file segments out to sendC2cFileMessage (was shipping "[空消息]")
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.
This commit is contained in:
@@ -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;
|
||||
|
||||
|
||||
@@ -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<ReturnType<typeof ref.bridge.sendPrivateMessage>> | 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 };
|
||||
|
||||
@@ -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> = {}): 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();
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user