diff --git a/packages/core/src/bridge/apis/contacts.ts b/packages/core/src/bridge/apis/contacts.ts index f3575a12..9ab3e151 100644 --- a/packages/core/src/bridge/apis/contacts.ts +++ b/packages/core/src/bridge/apis/contacts.ts @@ -1,3 +1,4 @@ +import { GroupInviteTracker } from '@snowluma/protocol/group-invite-tracker'; import { FetchDownloadRkeys } from '@snowluma/protocol/oidb-services/contacts/fetch-download-rkeys'; import { FetchFriendListPage, @@ -109,9 +110,7 @@ export class ContactsApi { load: (groupId) => this.fetchGroupMemberListUncached(groupId), }); private robotUinRangesPromise_: Promise | null = null; - /** groupUin → approval msgseq from a private qun.invite card. Written - * only by IncomingPacketPipeline (and tests). OneBot reads get/find. */ - private readonly groupInviteCardSeqs_ = new Map(); + readonly groupInvites = new GroupInviteTracker(); constructor(private readonly ctx: BridgeContext) {} @@ -616,32 +615,16 @@ export class ContactsApi { } /** Returns whether this live card introduces a new approval sequence. */ - rememberGroupInviteCardSequence(groupUin: number, sequence: number): boolean { - if (this.groupInviteCardSeqs_.get(groupUin) === sequence) { - return false; - } - - this.groupInviteCardSeqs_.set(groupUin, sequence); - return true; + rememberGroupInviteCardSequence(groupId: number, sequence: number): boolean { + return this.groupInvites.receiveCard(groupId, sequence); } - /** The approval msgseq captured from a private "qun.invite" card for this - * group, or undefined if none was seen. `set_group_add_request` uses it to - * approve a bot self-invite via 0x10c8 (eventType=2). See issue #125. */ getGroupInviteCardSequence(groupId: number): number | undefined { - return this.groupInviteCardSeqs_.get(groupId); + return this.groupInvites.getCardSequence(groupId); } - /** Resolve the group for a private invite-card msgseq. Numeric OneBot flags - * use this path because the card sequence is absent from 0x10C0. */ - findGroupInviteCardGroupBySequence(sequence: number): number | undefined { - for (const [groupUin, rememberedSequence] of this.groupInviteCardSeqs_) { - if (rememberedSequence === sequence) { - return groupUin; - } - } - - return undefined; + findGroupInviteCardGroupBySequence(sequence: number, groupId?: number): number | undefined { + return this.groupInvites.findCardGroup(sequence, groupId); } async fetchDownloadRKeys(): Promise { diff --git a/packages/core/src/bridge/bridge.ts b/packages/core/src/bridge/bridge.ts index 07183b88..a0d4e967 100644 --- a/packages/core/src/bridge/bridge.ts +++ b/packages/core/src/bridge/bridge.ts @@ -73,8 +73,7 @@ export class Bridge implements BridgeInterface { }, resolveGroupJoinRequest: (groupId, uid, subType, invitedUid) => this.apis.contacts.resolveGroupJoinRequest(groupId, uid, subType, invitedUid), - rememberGroupInviteCardSequence: (groupUin, sequence) => - this.apis.contacts.rememberGroupInviteCardSequence(groupUin, sequence), + groupInvites: this.apis.contacts.groupInvites, }); this.pipeline.registerCmd(MSG_PUSH_CMD, (pkt, identity) => parseMsgPushOrThrow(pkt, identity, this.sysMsgDedup_), diff --git a/packages/core/tests/actions/contacts.test.ts b/packages/core/tests/actions/contacts.test.ts index c5aaa8cd..29f76546 100644 --- a/packages/core/tests/actions/contacts.test.ts +++ b/packages/core/tests/actions/contacts.test.ts @@ -761,4 +761,13 @@ describe('apis/contacts / invite-card pending application', () => { expect(api.findGroupInviteCardGroupBySequence(778899)).toBe(12345); expect(api.findGroupInviteCardGroupBySequence(1)).toBeUndefined(); }); + + it('requires a group when different groups have the same card sequence', () => { + const api = new ContactsApi({ identity: { uin: '10001' } } as any); + api.rememberGroupInviteCardSequence(12345, 778899); + api.rememberGroupInviteCardSequence(67890, 778899); + + expect(() => api.findGroupInviteCardGroupBySequence(778899)).toThrow('multiple groups'); + expect(api.findGroupInviteCardGroupBySequence(778899, 67890)).toBe(67890); + }); }); diff --git a/packages/core/tests/msg-push-handler.test.ts b/packages/core/tests/msg-push-handler.test.ts index 49cbaf50..a5d9a14e 100644 --- a/packages/core/tests/msg-push-handler.test.ts +++ b/packages/core/tests/msg-push-handler.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it, vi } from 'vitest'; import { protobuf_encode } from '@snowluma/proton'; import { parseMsgPush, MSG_PUSH_CMD } from '@snowluma/protocol/msg-push'; import { IdentityService } from '@snowluma/protocol/identity-service'; +import { GroupInviteTracker } from '@snowluma/protocol/group-invite-tracker'; import { IncomingPacketPipeline } from '@snowluma/protocol/packet-pipeline'; import { BridgeEventBus } from '@snowluma/protocol/event-bus'; import type { GroupMemberInfo, QQGroupInfo } from '@snowluma/protocol/qq-info'; @@ -529,7 +530,7 @@ describe('parseMsgPush Event0x210 subType=364 (friend remark synchronization)', fetchGroupList: vi.fn(async () => {}), fetchGroupMemberList: vi.fn(async () => {}), resolveGroupJoinRequest: vi.fn(async () => null), - rememberGroupInviteCardSequence: vi.fn(() => true), + groupInvites: new GroupInviteTracker(), }); pipeline.registerCmd(MSG_PUSH_CMD, parseMsgPush); @@ -1028,7 +1029,7 @@ describe('parseMsgPush PkgType 44 (group admin set/unset) keeps the member cache fetchGroupList: vi.fn(async () => {}), fetchGroupMemberList: vi.fn(async () => {}), resolveGroupJoinRequest: vi.fn(async () => null), - rememberGroupInviteCardSequence: vi.fn(() => true), + groupInvites: new GroupInviteTracker(), }); pipeline.registerCmd(MSG_PUSH_CMD, parseMsgPush); await pipeline.process(packet); diff --git a/packages/onebot/src/modules/request-actions.ts b/packages/onebot/src/modules/request-actions.ts index 3adc6770..9a7212e7 100644 --- a/packages/onebot/src/modules/request-actions.ts +++ b/packages/onebot/src/modules/request-actions.ts @@ -125,7 +125,10 @@ async function resolveLiveGroupRequest( bridge: BridgeInterface, handle: GroupRequestHandle, ): Promise { - const cardGroup = bridge.apis.contacts.findGroupInviteCardGroupBySequence?.(handle.sequence); + const cardGroup = bridge.apis.contacts.findGroupInviteCardGroupBySequence?.( + handle.sequence, + handle.groupId, + ); // Private invite-card seq is absent from 0x10C0; keep the flag tuple (#125). if (cardGroup === handle.groupId) { @@ -231,28 +234,43 @@ export async function handleGroupAddRequest( // Legacy SnowLuma flags remain accepted for events emitted by older builds. // A private invite card needs its msgseq rather than the 0x10C0 tuple (#125). - if (parsed.requestType === 'invite') { - const cardSequence = bridge.apis.contacts.getGroupInviteCardSequence(parsed.groupId); + const notice = + parsed.requestType === 'invite' + ? bridge.apis.contacts.groupInvites.getSelfInviteNotice( + parsed.groupId, + parsed.targetUid, + ) + : undefined; - if (cardSequence) { - await applyGroupRequest( - bridge, - { - groupId: parsed.groupId, - sequence: cardSequence, - eventType: 2, - filtered: false, - }, - approve, - reason, - ); - return; - } + if (notice?.cardSequence) { + await applyGroupRequest( + bridge, + { + groupId: parsed.groupId, + sequence: notice.cardSequence, + eventType: 2, + filtered: false, + }, + approve, + reason, + ); + return; } const lookup = await fetchRequestInboxes(bridge, 'uid'); - const matching = lookup.requests.find((request) => { - if (request.groupId !== parsed.groupId) { + const candidates = lookup.requests.filter((request) => { + if (request.groupId !== parsed.groupId || request.state !== 1 || request.eventType <= 0) { + return false; + } + + if ( + notice && + (request.eventType !== 2 || + (request.targetUin > 0 && request.targetUin !== Number(bridge.identity.uin)) || + (request.targetUid && + bridge.identity.selfUid && + request.targetUid !== bridge.identity.selfUid)) + ) { return false; } @@ -261,11 +279,22 @@ export async function handleGroupAddRequest( : request.invitorUid === parsed.targetUid; }); - // Never fall back to an arbitrary request from the same group: when a UID - // does not match, approving another pending request is worse than failing. + const matching = candidates[0]; + if (!matching) { throwMissingGroupRequest('matching group request not found', lookup.failures); } + if ( + candidates.some( + (request) => + request.sequence !== matching.sequence || + request.eventType !== matching.eventType || + request.filtered !== matching.filtered, + ) + ) { + throw new Error('multiple pending group requests match this flag'); + } + await applyGroupRequest(bridge, matching, approve, reason, matching); } diff --git a/packages/onebot/tests/group-request-events.test.ts b/packages/onebot/tests/group-request-events.test.ts index f8c9af92..ba0dc460 100644 --- a/packages/onebot/tests/group-request-events.test.ts +++ b/packages/onebot/tests/group-request-events.test.ts @@ -70,11 +70,11 @@ function jsonCard(content: unknown, senderUin = 456): PacketInfo { }); } -function selfInviteNotice(): PacketInfo { +function selfInviteNotice(invitorUid = 'u_inviter', groupUin = 999): PacketInfo { return packet({ contentHead: { msgType: 87, timestamp: 1 }, body: { - msgContent: protobuf_encode({ groupUin: 999, invitorUid: 'u_inviter' }), + msgContent: protobuf_encode({ groupUin, invitorUid }), }, }); } @@ -131,7 +131,7 @@ describe('group request events', () => { ); it.each(['notice first', 'card first'])( - 'emits only the card when paired with a self-invite notice: %s', + 'emits one request when a self-invite notice and its card both arrive: %s', async (order) => { const { bridge, received, fetchRequests } = setup(); const packets = @@ -159,6 +159,116 @@ describe('group request events', () => { expect(second.received).toHaveLength(1); }); + it('pairs a notice exactly once and keeps its own card sequence for approval', async () => { + const { bridge, received, fetchRequests } = setup(); + bridge.identity.rememberRequestIdentity({ groupId: 999, uid: 'u_inviter', uin: 456 }); + const setAddRequest = vi.spyOn(bridge.apis.groupAdmin, 'setAddRequest').mockResolvedValue(); + await bridge.onPacket(selfInviteNotice()); + await bridge.onPacket(card(700)); + await bridge.onPacket(card(701)); + await bridge.onPacket(card(700)); + + expect(received.map((event) => event.flag)).toEqual([ + 'invite:999:u_inviter', + 'slreq:1:701:999:2:0', + ]); + await handleGroupAddRequest(bridge, received[0]!.flag, true, ''); + expect(setAddRequest).toHaveBeenCalledWith(999, 700, 2, true, '', false, undefined); + expect(fetchRequests).not.toHaveBeenCalled(); + }); + + it.each(['u_other', 'u_inviter'])( + 'does not approve a stale card for a new notice from %s', + async (uid) => { + vi.useFakeTimers({ toFake: ['Date'] }); + const { bridge, received, fetchRequests } = setup(); + bridge.identity.rememberRequestIdentity({ groupId: 999, uid, uin: 789 }); + const setAddRequest = vi + .spyOn(bridge.apis.groupAdmin, 'setAddRequest') + .mockResolvedValue(); + fetchRequests.mockImplementation(async (filtered) => + filtered + ? [] + : [request({ invitorUid: uid, sequence: 43, eventType: 2, targetUin: 10001 })], + ); + await bridge.onPacket(selfInviteNotice()); + await bridge.onPacket(card(700)); + vi.setSystemTime(Date.now() + 4_000); + await bridge.onPacket(selfInviteNotice(uid)); + + const latest = received.at(-1)!; + expect(latest.flag).toBe(`invite:999:${uid}`); + await handleGroupAddRequest(bridge, latest.flag, true, ''); + expect(setAddRequest).toHaveBeenCalledExactlyOnceWith( + 999, + 43, + 2, + true, + '', + false, + undefined, + ); + }, + ); + + it('does not emit a self-invite flag without a group and inviter', async () => { + const { bridge, received } = setup(); + await bridge.onPacket(selfInviteNotice('')); + await bridge.onPacket(selfInviteNotice(' ')); + await bridge.onPacket(selfInviteNotice('u_inviter', 0)); + expect(received).toEqual([]); + }); + + it.each([22, 2])( + 'cannot use a self-invitation flag to approve another account joining the group (type %i)', + async (eventType) => { + const { bridge, received, fetchRequests } = setup(); + bridge.identity.rememberRequestIdentity({ groupId: 999, uid: 'u_inviter', uin: 456 }); + const setAddRequest = vi + .spyOn(bridge.apis.groupAdmin, 'setAddRequest') + .mockResolvedValue(); + fetchRequests.mockImplementation(async (filtered) => + filtered ? [] : [request({ invitorUid: 'u_inviter', eventType, targetUin: 456 })], + ); + await bridge.onPacket(selfInviteNotice()); + await expect( + handleGroupAddRequest(bridge, received[0]!.flag, true, ''), + ).rejects.toThrow('matching group request not found'); + expect(setAddRequest).not.toHaveBeenCalled(); + }, + ); + + it('emits a later card when the pairing window has expired', async () => { + vi.useFakeTimers({ toFake: ['Date'] }); + const { bridge, received } = setup(); + bridge.identity.rememberRequestIdentity({ groupId: 999, uid: 'u_inviter', uin: 456 }); + await bridge.onPacket(selfInviteNotice()); + vi.setSystemTime(Date.now() + 4_000); + await bridge.onPacket(card(701)); + expect(received.map((event) => event.flag)).toEqual([ + 'invite:999:u_inviter', + 'slreq:1:701:999:2:0', + ]); + }); + + it('refuses an ambiguous legacy flag when two self invitations overlap', async () => { + const { bridge, received } = setup(); + bridge.identity.rememberRequestIdentity({ groupId: 999, uid: 'u_inviter', uin: 456 }); + const setAddRequest = vi.spyOn(bridge.apis.groupAdmin, 'setAddRequest').mockResolvedValue(); + await bridge.onPacket(selfInviteNotice()); + await bridge.onPacket(selfInviteNotice()); + await bridge.onPacket(card(701)); + await bridge.onPacket(card(702)); + expect(received.map((event) => event.flag)).toEqual([ + 'invite:999:u_inviter', + 'invite:999:u_inviter', + ]); + await expect(handleGroupAddRequest(bridge, received[0]!.flag, true, '')).rejects.toThrow( + 'multiple pending self invitations', + ); + expect(setAddRequest).not.toHaveBeenCalled(); + }); + it('does not report sent cards or links in unrelated JSON cards as invitations', async () => { const { bridge, received } = setup(); await bridge.onPacket(card(778899, 10001)); diff --git a/packages/onebot/tests/instance-context.test.ts b/packages/onebot/tests/instance-context.test.ts index d2c35ab5..3d41e8b5 100644 --- a/packages/onebot/tests/instance-context.test.ts +++ b/packages/onebot/tests/instance-context.test.ts @@ -200,7 +200,7 @@ function makeRef( fetchGroupRequests: vi.fn(async () => []), fetchDownloadRKeys: vi.fn(async () => []), findGroupInviteCardGroupBySequence: vi.fn(() => undefined), - getGroupInviteCardSequence: vi.fn(() => undefined), + groupInvites: { getSelfInviteNotice: vi.fn(() => undefined) }, ...overrides.apis?.contacts, }, groupFile: { diff --git a/packages/onebot/tests/request-actions.test.ts b/packages/onebot/tests/request-actions.test.ts index e493cb32..de1d3b29 100644 --- a/packages/onebot/tests/request-actions.test.ts +++ b/packages/onebot/tests/request-actions.test.ts @@ -12,7 +12,7 @@ const APIS_ROUTING: Record = { fetchGroupRequests: 'contacts', fetchGroupRequestsByUid: 'contacts', fetchDownloadRKeys: 'contacts', - getGroupInviteCardSequence: 'contacts', + groupInvites: 'contacts', findGroupInviteCardGroupBySequence: 'contacts', }; @@ -205,7 +205,7 @@ describe('onebot/modules/request-actions / handleGroupAddRequest', () => { filtered: false, }), ]), - getGroupInviteCardSequence: vi.fn(() => null), + groupInvites: { getSelfInviteNotice: vi.fn(() => undefined) }, apis: { groupAdmin: { setAddRequest } } as any, }); @@ -232,7 +232,7 @@ describe('onebot/modules/request-actions / handleGroupAddRequest', () => { ] : [], ), - getGroupInviteCardSequence: vi.fn(() => null), + groupInvites: { getSelfInviteNotice: vi.fn(() => undefined) }, apis: { groupAdmin: { setAddRequest } } as any, }); @@ -246,7 +246,7 @@ describe('onebot/modules/request-actions / handleGroupAddRequest', () => { it('surfaces "not found" only when neither inbox has the request', async () => { const bridge = fakeBridge({ fetchGroupRequestsByUid: vi.fn(async () => []), - getGroupInviteCardSequence: vi.fn(() => null), + groupInvites: { getSelfInviteNotice: vi.fn(() => undefined) }, apis: { groupAdmin: { setAddRequest: vi.fn(async () => {}) } } as any, }); await expect(handleGroupAddRequest(bridge, 'invite:999:u_i', true, 'ok')).rejects.toThrow( @@ -254,6 +254,44 @@ describe('onebot/modules/request-actions / handleGroupAddRequest', () => { ); }); + it('never picks one of two different pending invitations with the same legacy flag', async () => { + const setAddRequest = vi.fn(); + const bridge = fakeBridge({ + groupInvites: { getSelfInviteNotice: vi.fn(() => undefined) }, + fetchGroupRequestsByUid: vi.fn(async (filtered: boolean) => + filtered ? [] : [fakeRequest({ sequence: 123 }), fakeRequest({ sequence: 124 })], + ), + apis: { groupAdmin: { setAddRequest } }, + }); + await expect(handleGroupAddRequest(bridge, 'invite:999:u_i', true, '')).rejects.toThrow( + 'multiple pending group requests', + ); + expect(setAddRequest).not.toHaveBeenCalled(); + }); + + it('skips processed requests when approving a legacy notice', async () => { + const setAddRequest = vi.fn(); + const bridge = fakeBridge({ + groupInvites: { getSelfInviteNotice: vi.fn(() => undefined) }, + fetchGroupRequestsByUid: vi.fn(async (filtered: boolean) => + filtered + ? [] + : [fakeRequest({ sequence: 123, state: 2 }), fakeRequest({ sequence: 124 })], + ), + apis: { groupAdmin: { setAddRequest } }, + }); + await handleGroupAddRequest(bridge, 'invite:999:u_i', true, ''); + expect(setAddRequest).toHaveBeenCalledExactlyOnceWith( + 999, + 124, + 7, + true, + '', + false, + undefined, + ); + }); + it('does not treat an inviter QQ number as a request sequence (#213)', async () => { const setAddRequest = vi.fn(async () => {}); const bridge = fakeBridge({ diff --git a/packages/protocol/bench/recv.bench.ts b/packages/protocol/bench/recv.bench.ts index e6d6e391..86af4966 100644 --- a/packages/protocol/bench/recv.bench.ts +++ b/packages/protocol/bench/recv.bench.ts @@ -24,6 +24,7 @@ import type { GroupChange } from '@snowluma/proto-defs/notify'; import { parseMsgPush, MSG_PUSH_CMD } from '../src/msg-push/index'; import { IdentityService } from '../src/identity-service'; import { BridgeEventBus } from '../src/event-bus'; +import { GroupInviteTracker } from '../src/group-invite-tracker'; import { IncomingPacketPipeline } from '../src/packet-pipeline'; const SELF_UIN = '10001'; @@ -156,7 +157,7 @@ function makePipeline(identity: IdentityService): IncomingPacketPipeline { fetchGroupList: async () => {}, fetchGroupMemberList: async () => {}, resolveGroupJoinRequest: async () => null, - rememberGroupInviteCardSequence: () => true, + groupInvites: new GroupInviteTracker(), }); pipeline.registerCmd(MSG_PUSH_CMD, parseMsgPush); return pipeline; diff --git a/packages/protocol/package.json b/packages/protocol/package.json index 493f3b64..22502ce3 100644 --- a/packages/protocol/package.json +++ b/packages/protocol/package.json @@ -24,6 +24,10 @@ "types": "./src/identity-service.ts", "default": "./src/identity-service.ts" }, + "./group-invite-tracker": { + "types": "./src/group-invite-tracker.ts", + "default": "./src/group-invite-tracker.ts" + }, "./packet-pipeline": { "types": "./src/packet-pipeline.ts", "default": "./src/packet-pipeline.ts" diff --git a/packages/protocol/src/group-invite-tracker.ts b/packages/protocol/src/group-invite-tracker.ts new file mode 100644 index 00000000..a10f1f81 --- /dev/null +++ b/packages/protocol/src/group-invite-tracker.ts @@ -0,0 +1,142 @@ +const PAIR_WINDOW_MS = 3_000; +const MAX_INVITES = 1_024; + +interface Card { + groupId: number; + sequence: number; +} + +interface SelfInviteNotice { + cardSequence?: number; + ambiguous?: boolean; +} + +interface UnpairedInvite { + key: string; + notice?: SelfInviteNotice; + source: 'card' | 'notice'; + expiresAt: number; +} + +function remember(entries: Map, key: string, value: T): void { + entries.delete(key); + + if (entries.size >= MAX_INVITES) { + const oldest = entries.keys().next(); + + if (!oldest.done) { + entries.delete(oldest.value); + } + } + + entries.set(key, value); +} + +/** Account-local state shared by incoming invitations and their approval actions. */ +export class GroupInviteTracker { + private readonly cards = new Map(); + private readonly notices = new Map(); + private unpaired: UnpairedInvite[] = []; + + receiveNotice(groupId: number, uid: string): boolean { + const key = `${groupId}:${uid}`; + const notice: SelfInviteNotice = {}; + + if (this.pair(key, 'notice', notice)) { + return false; + } + + // Earlier notices with the same legacy flag cannot lend this one their card. + remember(this.notices, key, notice); + return true; + } + + receiveCard(groupId: number, sequence: number, uid?: string): boolean { + const key = `${groupId}:${sequence}`; + + if (this.cards.has(key)) { + return false; + } + + const paired = uid ? this.pair(`${groupId}:${uid}`, 'card') : undefined; + + if (paired?.notice) { + paired.notice.cardSequence = sequence; + } + + remember(this.cards, key, { groupId, sequence }); + return !paired; + } + + private pair( + key: string, + source: UnpairedInvite['source'], + notice?: SelfInviteNotice, + ): UnpairedInvite | undefined { + const now = Date.now(); + this.unpaired = this.unpaired.filter((entry) => entry.expiresAt > now); + const match = this.unpaired.findIndex( + (entry) => entry.key === key && entry.source !== source, + ); + + if (match !== -1) { + return this.unpaired.splice(match, 1)[0]; + } + + if (this.unpaired.length >= MAX_INVITES) { + this.unpaired.shift(); + } + + if ( + notice && + this.unpaired.some((entry) => entry.key === key && entry.source === 'notice') + ) { + notice.ambiguous = true; + } + + this.unpaired.push({ key, source, notice, expiresAt: now + PAIR_WINDOW_MS }); + return undefined; + } + + getSelfInviteNotice(groupId: number, uid: string): Readonly | undefined { + const notice = this.notices.get(`${groupId}:${uid}`); + + if (notice?.ambiguous) { + throw new Error('multiple pending self invitations share this flag'); + } + + return notice; + } + + getCardSequence(groupId: number): number | undefined { + let sequence: number | undefined; + + for (const card of this.cards.values()) { + if (card.groupId === groupId) { + sequence = card.sequence; + } + } + + return sequence; + } + + findCardGroup(sequence: number, groupId?: number): number | undefined { + if (groupId !== undefined) { + return this.cards.get(`${groupId}:${sequence}`)?.groupId; + } + + let match: number | undefined; + + for (const card of this.cards.values()) { + if (card.sequence === sequence) { + if (match !== undefined) { + throw new Error('multiple groups match this invitation card sequence'); + } + + match = card.groupId; + } + } + + return match; + } +} diff --git a/packages/protocol/src/packet-pipeline.ts b/packages/protocol/src/packet-pipeline.ts index 97cd6440..38553729 100644 --- a/packages/protocol/src/packet-pipeline.ts +++ b/packages/protocol/src/packet-pipeline.ts @@ -3,17 +3,10 @@ import { formatEvent } from './format'; import { createLogger, type Logger } from '@snowluma/common/logger'; import type { BridgeEventBus } from './event-bus'; import type { QQEventVariant } from './events'; +import type { GroupInviteTracker } from './group-invite-tracker'; import type { IdentityService } from './identity-service'; import { formatGroupRequestFlag, type GroupRequestInfo } from './qq-info'; -/** - * How long a dispatched invitation stays remembered so the MSF notice and the - * private `qun.invite` card of the SAME invitation do not both dispatch. - * Both packets come from one server-side action, so the window only bounds how - * long the marker lives -- matching itself is exact (group + inviter UID). - */ -const INVITE_PAIR_WINDOW_MS = 3_000; - const moduleLog = createLogger('Bridge'); const moduleEventLog = createLogger('Event'); const modulePacketLog = createLogger('Protocol.Packet'); @@ -60,8 +53,7 @@ export interface PacketPipelineDeps { subType: 'add' | 'invite', invitedUid?: string, ): Promise; - /** Remember a live card; false means its approval sequence was already seen. */ - rememberGroupInviteCardSequence(groupUin: number, sequence: number): boolean; + groupInvites: GroupInviteTracker; } export class IncomingPacketPipeline { @@ -70,15 +62,6 @@ export class IncomingPacketPipeline { private readonly log: Logger; private readonly eventLog: Logger; private readonly packetLog: Logger; - /** - * One invitation can arrive as an MSF notice AND as a private card. The card - * is the richer source (it carries the approval msgseq), so either one may - * dispatch -- but never both. Two independent markers keep that mutual: a - * notice suppresses the card that follows it, a card suppresses the notice. - * Cards never suppress each other (a new msgseq is a new invitation). - */ - private readonly noticeDispatched_ = new Map(); - private readonly cardDispatched_ = new Map(); constructor(private readonly deps: PacketPipelineDeps) { // Tag every line we emit with this Bridge's UIN so per-account file @@ -402,21 +385,11 @@ export class IncomingPacketPipeline { if ( !groupId || !sequence || - !this.deps.rememberGroupInviteCardSequence(groupId, sequence) + !this.deps.groupInvites.receiveCard(groupId, sequence, event.senderUid) ) { return false; } - // The msgseq is cached above either way. Skip the duplicate event only when - // the notice already delivered this same invitation -- a card with a new - // sequence is a genuinely new invitation and must always dispatch. - if (this.dispatchedRecently_(this.noticeDispatched_, groupId, event.senderUid ?? '')) { - this.log.debug('invitation already dispatched from the notice: group=%d', groupId); - return false; - } - - this.rememberDispatched_(this.cardDispatched_, groupId, event.senderUid ?? ''); - this.finishDispatch({ kind: 'group_invite', time: event.time, @@ -432,27 +405,6 @@ export class IncomingPacketPipeline { return true; } - private dispatchedRecently_( - marker: Map, - groupId: number, - inviterUid: string, - ): boolean { - const key = `${groupId}:${inviterUid}`; - const at = marker.get(key); - if (at === undefined) return false; - if (Date.now() - at < INVITE_PAIR_WINDOW_MS) return true; - marker.delete(key); - return false; - } - - private rememberDispatched_( - marker: Map, - groupId: number, - inviterUid: string, - ): void { - marker.set(`${groupId}:${inviterUid}`, Date.now()); - } - private emit(event: QQEventVariant): void { // Fire-and-forget: errors inside subscribers are surfaced via the bus's // own onError hook so one bad listener never blocks the others. @@ -550,22 +502,17 @@ export class IncomingPacketPipeline { pkt: PacketInfo, event: Extract, ): Promise { - // A self-invite never appears in the UIN-based request inbox, so there is - // nothing to enrich from and querying it only wastes a round trip. The - // private card carries the approval msgseq, but it only arrives for some - // inviters -- when it does not, the notice is the only chance to surface - // the invitation at all. Dispatch it with the decoder-provided legacy flag, - // and let the card win when both arrive (see `dispatchedInvites_`). + // A self-invite notice carries an inviter UID but no approval sequence. if ( event.subType === 'invite' && this.isSelfMemberIdentity(event.invitedUin ?? 0, event.invitedUid) ) { - if (!event.flag) { - this.log.warn('self invitation without a flag: group=%d', event.groupId); + if (!(event.groupId > 0) || !event.fromUid?.trim() || !event.flag) { + this.log.warn('self invitation missing group or inviter: group=%d', event.groupId); return 0; } - if (this.dispatchedRecently_(this.cardDispatched_, event.groupId, event.fromUid ?? '')) { + if (!this.deps.groupInvites.receiveNotice(event.groupId, event.fromUid)) { this.log.debug( 'self invitation already dispatched from the card: group=%d', event.groupId, @@ -573,7 +520,18 @@ export class IncomingPacketPipeline { return 0; } - this.rememberDispatched_(this.noticeDispatched_, event.groupId, event.fromUid ?? ''); + if (!event.fromUin) { + event.fromUin = + (await this.deps.identity.resolveUin(event.fromUid, event.groupId)) ?? 0; + + if (!event.fromUin) { + this.log.warn( + 'self invitation sender unavailable: group=%d uid=%s', + event.groupId, + event.fromUid, + ); + } + } try { return this.dispatchEvent(pkt, event, 'sync'); diff --git a/packages/protocol/tests/packet-pipeline-roster-refresh.test.ts b/packages/protocol/tests/packet-pipeline-roster-refresh.test.ts index 45f474c3..de015319 100644 --- a/packages/protocol/tests/packet-pipeline-roster-refresh.test.ts +++ b/packages/protocol/tests/packet-pipeline-roster-refresh.test.ts @@ -3,6 +3,7 @@ import type { PacketInfo } from '@snowluma/common/protocol-types'; import { BridgeEventBus } from '@snowluma/protocol/event-bus'; import type { QQEventVariant } from '@snowluma/protocol/events'; import { IdentityService } from '@snowluma/protocol/identity-service'; +import { GroupInviteTracker } from '@snowluma/protocol/group-invite-tracker'; import { IncomingPacketPipeline } from '@snowluma/protocol/packet-pipeline'; import type { GroupMemberInfo, QQGroupInfo, UserProfileInfo } from '@snowluma/protocol/qq-info'; import { afterEach, describe, expect, it, vi } from 'vitest'; @@ -154,7 +155,7 @@ function makePipeline( fetchGroupList, fetchGroupMemberList, resolveGroupJoinRequest: vi.fn(async () => null), - rememberGroupInviteCardSequence: vi.fn(() => true), + groupInvites: new GroupInviteTracker(), }); return { identity, pipeline, captured, fetchGroupList, fetchGroupMemberList }; } diff --git a/packages/protocol/tests/packet-pipeline-stranger-resolve.test.ts b/packages/protocol/tests/packet-pipeline-stranger-resolve.test.ts index a3e7ada5..c57add7c 100644 --- a/packages/protocol/tests/packet-pipeline-stranger-resolve.test.ts +++ b/packages/protocol/tests/packet-pipeline-stranger-resolve.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it, vi } from 'vitest'; +import { GroupInviteTracker } from '@snowluma/protocol/group-invite-tracker'; import { IncomingPacketPipeline } from '@snowluma/protocol/packet-pipeline'; import { BridgeEventBus } from '@snowluma/protocol/event-bus'; import { IdentityService } from '@snowluma/protocol/identity-service'; @@ -49,7 +50,7 @@ function makePipeline( fetchGroupList: vi.fn(async () => {}), fetchGroupMemberList: vi.fn(async () => {}), resolveGroupJoinRequest, - rememberGroupInviteCardSequence: vi.fn(() => true), + groupInvites: new GroupInviteTracker(), }); const captured: QQEventVariant[] = []; @@ -193,23 +194,32 @@ describe('IncomingPacketPipeline / group requests', () => { ]); }); - it('dispatches a self invitation without querying the inbox', async () => { - // A self-invite never shows up in the UIN-based inbox, so the lookup is - // skipped and the decoder-provided legacy flag is what keeps it actionable. - const { pipeline, captured, resolveGroupJoinRequest, fetchProfileByUid } = makePipeline(); - await receive( - pipeline, - requestEvent({ subType: 'invite', invitedUin: 10001, flag: 'invite:12345:u_requester' }), - ); - expect(captured).toHaveLength(1); - expect(captured[0]).toMatchObject({ - kind: 'group_invite', - invitedUin: 10001, - flag: 'invite:12345:u_requester', - }); - expect(resolveGroupJoinRequest).not.toHaveBeenCalled(); - expect(fetchProfileByUid).not.toHaveBeenCalled(); - }); + it.each([76543, 0])( + 'resolves the self-invite sender without querying the inbox (profile UIN %d)', + async (uin) => { + const fetchProfileByUid = vi.fn(async () => makeProfile(uin)); + const { pipeline, captured, resolveGroupJoinRequest } = makePipeline({ + fetchProfileByUid, + }); + await receive( + pipeline, + requestEvent({ + subType: 'invite', + invitedUin: 10001, + flag: 'invite:12345:u_requester', + }), + ); + expect(captured).toHaveLength(1); + expect(captured[0]).toMatchObject({ + kind: 'group_invite', + fromUin: uin, + invitedUin: 10001, + flag: 'invite:12345:u_requester', + }); + expect(resolveGroupJoinRequest).not.toHaveBeenCalled(); + expect(fetchProfileByUid).toHaveBeenCalledExactlyOnceWith('u_requester'); + }, + ); it('does not substitute a list sequence for the self-invite card sequence', async () => { const resolveGroupJoinRequest = vi.fn(async () => @@ -289,7 +299,7 @@ describe('IncomingPacketPipeline / #1 group card self-heal', () => { fetchGroupList: vi.fn(async () => {}), fetchGroupMemberList: vi.fn(async () => {}), resolveGroupJoinRequest: vi.fn(async () => null), - rememberGroupInviteCardSequence: vi.fn(() => true), + groupInvites: new GroupInviteTracker(), }); return { identity, pipeline }; } @@ -403,7 +413,7 @@ describe('IncomingPacketPipeline / group_card_change from message traffic', () = fetchGroupList: vi.fn(async () => {}), fetchGroupMemberList: vi.fn(async () => {}), resolveGroupJoinRequest: vi.fn(async () => null), - rememberGroupInviteCardSequence: vi.fn(() => true), + groupInvites: new GroupInviteTracker(), }); const captured: QQEventVariant[] = []; events.onAny((e) => captured.push(e as QQEventVariant)); diff --git a/packages/protocol/tests/packet-pipeline-trace.test.ts b/packages/protocol/tests/packet-pipeline-trace.test.ts index 2fbd5d37..64561c38 100644 --- a/packages/protocol/tests/packet-pipeline-trace.test.ts +++ b/packages/protocol/tests/packet-pipeline-trace.test.ts @@ -13,6 +13,7 @@ import { IdentityService } from '@snowluma/protocol/identity-service'; import type { MsgPushContext } from '@snowluma/protocol/msg-push/context'; import { PkgType } from '@snowluma/protocol/msg-push/enums'; import { MsgPushRegistry } from '@snowluma/protocol/msg-push/registry'; +import { GroupInviteTracker } from '@snowluma/protocol/group-invite-tracker'; import { IncomingPacketPipeline } from '@snowluma/protocol/packet-pipeline'; import { afterEach, describe, expect, it, vi } from 'vitest'; @@ -140,7 +141,7 @@ function makePipeline( comment: '', filtered: false, })), - rememberGroupInviteCardSequence: vi.fn(() => true), + groupInvites: new GroupInviteTracker(), }); const dispatched: Array<{ event: QQEventVariant; req: number | undefined }> = []; events.onAny((event) => { diff --git a/packages/protocol/tests/self-invite-dispatch.test.ts b/packages/protocol/tests/self-invite-dispatch.test.ts index 57407144..90f66115 100644 --- a/packages/protocol/tests/self-invite-dispatch.test.ts +++ b/packages/protocol/tests/self-invite-dispatch.test.ts @@ -2,6 +2,7 @@ import type { PacketInfo } from '@snowluma/common/protocol-types'; import { BridgeEventBus } from '@snowluma/protocol/event-bus'; import type { GroupInviteEvent, QQEventVariant } from '@snowluma/protocol/events'; import { IdentityService } from '@snowluma/protocol/identity-service'; +import { GroupInviteTracker } from '@snowluma/protocol/group-invite-tracker'; import { IncomingPacketPipeline } from '@snowluma/protocol/packet-pipeline'; import { afterEach, describe, expect, it, vi } from 'vitest'; @@ -103,7 +104,7 @@ function makePipeline(options: { request?: 'found' | 'missing' } = {}) { fetchGroupList: vi.fn(async () => {}), fetchGroupMemberList: vi.fn(async () => {}), resolveGroupJoinRequest, - rememberGroupInviteCardSequence: vi.fn(() => true), + groupInvites: new GroupInviteTracker(), }); return { pipeline, received, resolveGroupJoinRequest };