From 04523e252ff10d9a910ae50de5807fc0a324e44b Mon Sep 17 00:00:00 2001 From: Eric-GoodBoy-Tech <634718930@qq.com> Date: Thu, 2 Jul 2026 22:25:47 +0800 Subject: [PATCH 01/10] fix(qqbot): markdown-first send with replyMsgId TTL and dead code removal - Change replyMsgId from Map to Map with 5-minute TTL and periodic cleanup timer - Add setReplyMsgId() helper with cascaded msgSeqMap cleanup - Rewrite sendMessage(): markdown-first (msg_type:2), active retry on non-4xx failure, plain-text fallback for active messages - Re-throw errors for .catch() callers instead of silent break - Update restoreQQState() with backward-compatible replyMsgId migration - Remove dead code exports: hasMarkdownSyntax, hasLinkSyntax, splitText - Update send.test.ts: TTL expiry, markdown fallback, noreply suppression, replyMsgId helper tests --- packages/channels/qqbot/src/QQChannel.ts | 279 +++++++++------ packages/channels/qqbot/src/send.test.ts | 435 ++++++++++++++--------- 2 files changed, 439 insertions(+), 275 deletions(-) diff --git a/packages/channels/qqbot/src/QQChannel.ts b/packages/channels/qqbot/src/QQChannel.ts index b3c95d63e4f..2b5c3a6852a 100644 --- a/packages/channels/qqbot/src/QQChannel.ts +++ b/packages/channels/qqbot/src/QQChannel.ts @@ -59,53 +59,6 @@ export function isValidChatId(id: string): boolean { return /^[A-Za-z0-9_-]+$/.test(id) && id.length <= 128; } -/** - * Detect whether text contains markdown syntax (for msg_type selection). - * - * The list-item patterns `^[-*+]\s` and `^\d+\.\s` trade precision for recall: - * text like "- temperature: 5°C" or "1. first thing" will trigger markdown - * mode. Sending non-markdown as msg_type=2 (markdown) is harmless — QQ renders - * it as plain text — so false positives are safe. False negatives (missing - * markdown in msg_type=0) would strip formatting, so we bias toward markdown. - */ -export function hasLinkSyntax(text: string): boolean { - const open = text.indexOf('['); - if (open === -1) return false; - const mid = text.indexOf('](', open + 1); - if (mid === -1) return false; - return text.indexOf(')', mid + 2) !== -1; -} - -export function hasMarkdownSyntax(text: string): boolean { - return ( - /^#{1,6}\s/m.test(text) || - text.includes('```') || - /\*\*|__|~~/.test(text) || - /`[^`]+`/.test(text) || - hasLinkSyntax(text) || - /^[-*+]\s/m.test(text) || - /^\d+\.\s/m.test(text) - ); -} - -/** - * Split long text into QQ-compatible chunks (max 2000 chars each). - * - * Uses UTF-16 code-unit length — in the extremely rare case that the - * 2000-unit boundary falls in the middle of a surrogate pair (emoji), - * that character will be garbled. QQ chat messages rarely approach - * this limit at a boundary that aligns with a high-codepoint character. - */ -export function splitText(text: string): string[] { - const MAX = 2000; - if (text.length <= MAX) return [text]; - const chunks: string[] = []; - for (let i = 0; i < text.length; i += MAX) { - chunks.push(text.slice(i, i + MAX)); - } - return chunks; -} - export class QQChannel extends ChannelBase { private ws: WebSocket | null = null; private accessToken: string = ''; @@ -145,9 +98,12 @@ export class QQChannel extends ChannelBase { /** Track whether a chatId is a group or C2C for correct API routing. */ private chatTypeMap: Map = new Map(); /** Track the latest user messageId per chatId for proper reply (msg_id). */ - private replyMsgId: Map = new Map(); + private replyMsgId: Map = + new Map(); /** msg_seq counter per user messageId, for multi-block streaming. */ private msgSeqMap: Map = new Map(); + /** Periodic cleanup timer for expired replyMsgId entries. */ + private replyMsgIdCleanupTimer: ReturnType | null = null; /** Path to persisted QQ routing state: chatTypeMap, replyMsgId, msgSeqMap. */ private readonly qqStatePath: string; @@ -216,6 +172,7 @@ export class QQChannel extends ChannelBase { } this.beforeExitHook = () => this.flushQQState(); process.on('beforeExit', this.beforeExitHook); + this.startReplyMsgIdCleanup(); return; } catch (e: unknown) { if (attempt < 2) { @@ -238,81 +195,127 @@ export class QQChannel extends ChannelBase { } async sendMessage(chatId: string, text: string): Promise { - // ── Normal text / markdown flow ────────────────────────── + // suppression + if (text.trim() === '') { + process.stderr.write( + `[QQ:${this.name}] skipped for ${sanitizeLogText(chatId, 64)}\n`, + ); + return; + } + const route = await this.resolveRoute(chatId); if (!route) return; - const msgId = this.replyMsgId.get(chatId); - const useMarkdown = hasMarkdownSyntax(text); + // Look up reply context with TTL check + const entry = this.replyMsgId.get(chatId); + const msgId = + entry && Date.now() - entry.timestamp < 300_000 ? entry.msgId : undefined; + if (entry && !msgId) { + process.stderr.write( + `[QQ:${this.name}] replyMsgId entry expired for ${sanitizeLogText(chatId, 64)}, falling back to active message\n`, + ); + } + + let nextSeq = 0; + let rollbackApplied = false; + try { + // Try markdown first (msg_type: 2) + const body: Record = { + msg_type: 2, + markdown: { content: text }, + }; + nextSeq = msgId ? (this.msgSeqMap.get(msgId) ?? 0) + 1 : 0; + if (msgId) { + this.msgSeqMap.set(msgId, nextSeq); + body['msg_id'] = msgId; + body['msg_seq'] = nextSeq; + } - for (const chunk of splitText(text)) { - try { - const body: Record = useMarkdown - ? { msg_type: 2, markdown: { content: chunk } } - : { content: chunk, msg_type: 0 }; - // Multi-block streaming: set msg_id + incrementing msg_seq - // seq incremented before send so we can track the next value - const nextSeq = msgId ? (this.msgSeqMap.get(msgId) ?? 0) + 1 : 0; - if (msgId) { - body['msg_id'] = msgId; - body['msg_seq'] = nextSeq; - } + const resp = await sendQQMessage( + route.base, + route.path, + this.accessToken, + body, + ); - let resp = await sendQQMessage( - route.base, - route.path, - this.accessToken, - body, + if (!resp.ok) { + const errBody = await resp.text().catch(() => ''); + process.stderr.write( + `[QQ:${this.name}] Markdown rejected (HTTP ${resp.status}: ${sanitizeLogText(errBody, 200)})\n`, ); - // Markdown is a fully available, zero-permission message type on the QQ - // Bot Open Platform — bot.q.qq.com API docs list msg_type=2 alongside - // text/ark/embed with no application gate. (q.qq.com/wiki/FAQ/robot - // mentions a markdown permission application, but that FAQ targets a - // different platform — likely older 群机器人 or mini-program bots — - // not the Open Platform API we use here.) We retry as plaintext as - // defense-in-depth against edge cases where a bot's markdown capability - // might be restricted server-side. - if (!resp.ok && useMarkdown) { - const errBody = await resp.text().catch(() => ''); - process.stderr.write( - `[QQ:${this.name}] Markdown rejected (HTTP ${resp.status}: ${sanitizeLogText(errBody, 200)}), retrying as plain text\n`, - ); - const plainBody: Record = { - content: chunk, + if (msgId) { + this.msgSeqMap.set(msgId, nextSeq - 1); + rollbackApplied = true; + const activeBody: Record = { + content: text, msg_type: 0, + msg_id: msgId, + msg_seq: nextSeq + 1, }; - if (msgId) { - plainBody['msg_id'] = msgId; - plainBody['msg_seq'] = nextSeq; - } - resp = await sendQQMessage( + const activeResp = await sendQQMessage( route.base, route.path, this.accessToken, - plainBody, + activeBody, + ); + if (activeResp.ok) { + this.msgSeqMap.set(msgId, nextSeq + 1); + this.saveQQState(); + return; + } + process.stderr.write( + `[QQ:${this.name}] Active retry also failed (HTTP ${activeResp.status}: ${sanitizeLogText(await activeResp.text().catch(() => ''), 200)})\n`, ); + if (activeResp.status === 429) { + process.stderr.write( + `[QQ:${this.name}] Active retry rate-limited (HTTP 429), giving up\n`, + ); + this.saveQQState(); + return; + } } - if (!resp.ok) { - // Drain response body to avoid socket leak - const errBody = await resp.text().catch(() => ''); + // Active retry failed — skip plain-text fallback for passive replies + if (msgId) return; + + // Plain-text fallback for active messages (no reply context) + const plainBody: Record = { + content: text, + msg_type: 0, + }; + const fallbackRes = await sendQQMessage( + route.base, + route.path, + this.accessToken, + plainBody, + ); + if (!fallbackRes.ok) { process.stderr.write( - `[QQ:${this.name}] Send HTTP ${resp.status} (msg_seq=${body['msg_seq'] ?? '-'}): ${sanitizeLogText(errBody, 200)}\n`, + `[QQ:${this.name}] Plain-text fallback failed: ${fallbackRes.status}\n`, ); - break; // stop sending on failure to avoid msg_seq gaps } - // Only persist seq on success - if (msgId) this.msgSeqMap.set(msgId, nextSeq); - } catch (e) { + return; + } + + // Success — persist msgSeq + if (msgId) this.saveQQState(); + } catch (e) { + // Rollback on failure if we haven't already + if (msgId && !rollbackApplied) { + this.msgSeqMap.set(msgId, nextSeq - 1); + this.saveQQState(); + } + if (String(e).includes('429')) { process.stderr.write( - `[QQ:${this.name}] Send error: ${sanitizeLogText(e instanceof Error ? e.message : String(e), 200)}\n`, + `[QQ:${this.name}] MESSAGE DROPPED: rate-limited (429) on all send attempts for ${sanitizeLogText(chatId, 64)}\n`, ); - break; } + process.stderr.write( + `[QQ:${this.name}] Send error: ${sanitizeLogText(e instanceof Error ? e.message : String(e), 200)}\n`, + ); + throw e; // Re-throw for .catch() callers } - // Persist msgSeqMap once after all chunks are sent - if (msgId) this.saveQQState(); } /** @@ -343,6 +346,7 @@ export class QQChannel extends ChannelBase { this.disposed = true; this.stopHeartbeat(); this.stopTokenRefresh(); + this.stopReplyMsgIdCleanup(); if (this.seenCleanupTimer) { clearInterval(this.seenCleanupTimer); this.seenCleanupTimer = null; @@ -487,17 +491,27 @@ export class QQChannel extends ChannelBase { ); } if (raw.replyMsgId && Array.isArray(raw.replyMsgId)) { + const now = Date.now(); const rawRM = raw.replyMsgId as Array<[string, unknown]>; - // Validate: entries must be strings ≤ 128 chars this.replyMsgId = new Map( - rawRM.filter( - ([k, v]) => - typeof k === 'string' && - k.length <= 256 && - typeof v === 'string' && - v.length <= 128, - ), - ) as Map; + rawRM + .map(([k, v]) => + // Old format: string -> { msgId: v, timestamp: now } + // New format: { msgId, timestamp } -> pass through + typeof v === 'string' + ? ([k, { msgId: v, timestamp: now }] as const) + : ([k, v] as const), + ) + .filter(([k, v]) => { + if (typeof k !== 'string' || k.length > 256) return false; + if (typeof v !== 'object' || v === null) return false; + const entry = v as { msgId?: unknown; timestamp?: unknown }; + return ( + typeof entry.msgId === 'string' && + typeof entry.timestamp === 'number' + ); + }), + ) as Map; const dropped = rawRM.length - this.replyMsgId.size; if (dropped > 0) process.stderr.write( @@ -609,6 +623,47 @@ export class QQChannel extends ChannelBase { } } + // ── ReplyMsgId helpers ──────────────────────────────────────── + + /** + * Set replyMsgId for a chat, cleaning up the previous entry's msgSeqMap + * to prevent orphaned entries accumulating over time. + */ + private setReplyMsgId(chatId: string, msgId: string): void { + const oldEntry = this.replyMsgId.get(chatId); + if (oldEntry) { + this.msgSeqMap.delete(oldEntry.msgId); + } + this.replyMsgId.set(chatId, { msgId, timestamp: Date.now() }); + this.saveQQState(); + } + + /** + * Start periodic cleanup of expired replyMsgId entries. + * Evicts entries older than 5 minutes every 60 seconds, and cascades + * to msgSeqMap. + */ + private startReplyMsgIdCleanup(): void { + this.stopReplyMsgIdCleanup(); + this.replyMsgIdCleanupTimer = setInterval(() => { + const cutoff = Date.now() - 300_000; + for (const [chatId, entry] of this.replyMsgId) { + if (entry.timestamp < cutoff) { + this.msgSeqMap.delete(entry.msgId); + this.replyMsgId.delete(chatId); + } + } + }, 60_000); + this.replyMsgIdCleanupTimer.unref(); + } + + private stopReplyMsgIdCleanup(): void { + if (this.replyMsgIdCleanupTimer) { + clearInterval(this.replyMsgIdCleanupTimer); + this.replyMsgIdCleanupTimer = null; + } + } + // ── Token ────────────────────────────────────────────────────── private async fetchToken(): Promise { @@ -1043,7 +1098,7 @@ export class QQChannel extends ChannelBase { // not expose a unified user identity, so this is unavoidable. const chatId = event.author.user_openid || event.author.id; this.chatTypeMap.set(chatId, 'c2c'); - this.replyMsgId.set(chatId, event.id); + this.setReplyMsgId(chatId, event.id); this.saveQQState(); this.handleInbound({ channelName: this.name, @@ -1072,7 +1127,7 @@ export class QQChannel extends ChannelBase { } const chatId = event.group_openid; this.chatTypeMap.set(chatId, 'group'); - this.replyMsgId.set(chatId, event.id); + this.setReplyMsgId(chatId, event.id); this.saveQQState(); const senderName = event.author.username || event.author.id || 'QQ User'; // Strip @mention tags from message content. QQ Bot API docs state the API diff --git a/packages/channels/qqbot/src/send.test.ts b/packages/channels/qqbot/src/send.test.ts index 32c15f40769..db8c7e0cc7a 100644 --- a/packages/channels/qqbot/src/send.test.ts +++ b/packages/channels/qqbot/src/send.test.ts @@ -3,7 +3,7 @@ import type { ChannelAgentBridge, ChannelTaskLifecycleEvent, } from '@qwen-code/channel-base'; -import { isValidChatId, hasMarkdownSyntax, splitText } from './QQChannel.js'; +import { isValidChatId } from './QQChannel.js'; const { mockSendQQMessage, @@ -82,10 +82,6 @@ vi.mock('./login.js', () => ({ })); vi.mock('@qwen-code/channel-base', async () => { - // Pull the REAL sanitizeSenderName from the shared helper so a trojan-source - // or control-char regression is caught here, not masked by a stub. The vitest - // config aliases @qwen-code/channel-base to its SOURCE, so this resolves with - // no prior channel-base build (dist may be absent/stale in package-local runs). const real = await vi.importActual( '@qwen-code/channel-base', ); @@ -121,8 +117,6 @@ vi.mock('@qwen-code/channel-base', async () => { getGlobalQwenDir: () => '/tmp/test-qwen', sanitizeSenderName: real.sanitizeSenderName, sanitizePromptText: real.sanitizePromptText, - // Use the REAL log sanitizer so the audit-log hygiene test exercises the - // shared strip set (C0/DEL + PROMPT_UNSAFE_INVISIBLES), not a stub. sanitizeLogText: real.sanitizeLogText, }; }); @@ -191,100 +185,6 @@ describe('isValidChatId', () => { }); }); -describe('hasMarkdownSyntax', () => { - it('detects headings', () => { - expect(hasMarkdownSyntax('# Title')).toBe(true); - expect(hasMarkdownSyntax('## Subtitle')).toBe(true); - expect(hasMarkdownSyntax('###### Deep heading')).toBe(true); - }); - - it('detects code blocks', () => { - expect(hasMarkdownSyntax('```js\ncode\n```')).toBe(true); - }); - - it('detects bold (double asterisk)', () => { - expect(hasMarkdownSyntax('**bold**')).toBe(true); - }); - - it('detects bold (double underscore)', () => { - expect(hasMarkdownSyntax('__bold__')).toBe(true); - }); - - it('detects strikethrough', () => { - expect(hasMarkdownSyntax('~~strikethrough~~')).toBe(true); - }); - - it('detects inline code', () => { - expect(hasMarkdownSyntax('use `code` here')).toBe(true); - }); - - it('detects links', () => { - expect(hasMarkdownSyntax('[text](url)')).toBe(true); - }); - - it('detects unordered list markers', () => { - expect(hasMarkdownSyntax('- item')).toBe(true); - expect(hasMarkdownSyntax('* item')).toBe(true); - expect(hasMarkdownSyntax('+ item')).toBe(true); - }); - - it('detects ordered list markers', () => { - expect(hasMarkdownSyntax('1. first')).toBe(true); - expect(hasMarkdownSyntax('123. item')).toBe(true); - }); - - it('returns false for plain text', () => { - expect(hasMarkdownSyntax('hello world')).toBe(false); - expect(hasMarkdownSyntax('no special chars here')).toBe(false); - }); - - it('returns false for text with single asterisks (not list marker at line start)', () => { - expect(hasMarkdownSyntax('this is *not* italic in this regex')).toBe(false); - }); - - it('false positive: "- temperature" triggers list pattern', () => { - expect(hasMarkdownSyntax('- temperature: 5°C')).toBe(true); - }); - - it('false positive: "1. first thing" at line start triggers ordered-list pattern', () => { - expect(hasMarkdownSyntax('1. first thing in sentence')).toBe(true); - }); -}); - -describe('splitText', () => { - it('returns single-element array for short text', () => { - expect(splitText('hello')).toEqual(['hello']); - }); - - it('returns single-element array for exactly 2000 chars', () => { - const text = 'a'.repeat(2000); - const result = splitText(text); - expect(result).toHaveLength(1); - expect(result[0]).toHaveLength(2000); - }); - - it('splits text longer than 2000 chars into chunks', () => { - const text = 'a'.repeat(4500); - const result = splitText(text); - expect(result).toHaveLength(3); - expect(result[0]).toHaveLength(2000); - expect(result[1]).toHaveLength(2000); - expect(result[2]).toHaveLength(500); - }); - - it('preserves content across chunk boundaries', () => { - const text = 'x'.repeat(2000) + 'y'.repeat(500); - const result = splitText(text); - expect(result).toHaveLength(2); - expect(result[0]).toBe('x'.repeat(2000)); - expect(result[1]).toBe('y'.repeat(500)); - }); - - it('handles empty string', () => { - expect(splitText('')).toEqual(['']); - }); -}); - describe('session persistence paths', () => { function makeChannel( name: string, @@ -377,8 +277,6 @@ describe('group sender-name sanitization', () => { } it('neutralizes a crafted nickname (brackets, newline, >64 chars) before self-prefixing', () => { - // Fake timers so isDuplicate's eviction interval / saveQQState debounce don't - // leak past the test. vi.useFakeTimers(); const ch = makeChannel(); const inbound = vi.fn().mockResolvedValue(undefined); @@ -399,16 +297,13 @@ describe('group sender-name sanitization', () => { text: string; alreadyPrefixed?: boolean; }; - // No newline escapes the tag, and only the wrapper's own [ ] survive. expect(env.text).not.toContain('\n'); expect((env.text.match(/[[\]]/g) ?? []).length).toBe(2); - // The nick inside the tag is capped at 64 chars. const inside = env.text.slice( env.text.indexOf('[') + 1, env.text.indexOf(']'), ); expect(inside.length).toBeLessThanOrEqual(64); - // Normal (non-slash) group messages stay self-prefixed. expect(env.alreadyPrefixed).toBe(true); expect(env.text).toContain('hello world'); }); @@ -438,8 +333,6 @@ describe('group sender-name sanitization', () => { }); it('passes a group slash command through verbatim without the [sender] tag or alreadyPrefixed', () => { - // Fake timers so isDuplicate's eviction interval / saveQQState debounce don't - // leak past the test. vi.useFakeTimers(); const ch = makeChannel(); const inbound = vi.fn().mockResolvedValue(undefined); @@ -459,22 +352,11 @@ describe('group sender-name sanitization', () => { text: string; alreadyPrefixed?: boolean; }; - // The slash command is forwarded raw — no [Alice] prefix would let it parse - // as a command, so the cleanText must arrive untouched. expect(env.text).toBe('/clear'); - // And alreadyPrefixed must NOT be set: setting it would route the command - // through ChannelBase as already-attributed text. A regression that always - // sets alreadyPrefixed is caught here. expect(env.alreadyPrefixed).toBeUndefined(); }); it('sanitizes the sender name AND command text in the slash-command audit log (no log forging)', () => { - // event.author.username and content are attacker-controlled. The slash-command - // audit log must use the sanitized name and a neutralized command string, so a - // crafted QQ nick/message with CR/LF or ANSI escapes can't forge or corrupt the - // operator audit trail. Mutation check: logging the RAW senderName/cleanText - // (the pre-fix code) lets the ESC and the injected newline through and fails the - // assertions below. vi.useFakeTimers(); const ch = makeChannel(); (ch as unknown as { handleInbound: () => Promise }).handleInbound = @@ -490,11 +372,6 @@ describe('group sender-name sanitization', () => { }); const ESC = String.fromCharCode(0x1b); - // NEL (U+0085) is a Unicode line break and U+009B a C1 CSI introducer: both are - // attacker-controlled C1 chars that must be neutralized like ESC/CR, or a raw - // NEL would render as a line break and forge a second audit entry. U+2028 (line - // separator) likewise renders as a break and U+202E (bidi RTL override) reorders - // the line (trojan-source) — both covered by the shared log sanitizer. const NEL = String.fromCharCode(0x85); const C1 = String.fromCharCode(0x9b); const LS = String.fromCharCode(0x2028); @@ -510,26 +387,14 @@ describe('group sender-name sanitization', () => { const audit = writes.find((w) => w.includes('Slash cmd from')); expect(audit).toBeDefined(); - // No ANSI escape survives in the log line. expect(audit!.includes(ESC)).toBe(false); - // The only newline is the log line's own trailing one — no injected break from - // the nick or command text (which would forge a second audit entry). expect(audit!.split('\n')).toHaveLength(2); expect(audit!.endsWith('\n')).toBe(true); - // The raw (unsanitized) nick fragment never appears verbatim. expect(audit!.includes(`Ev${ESC}`)).toBe(false); - // The C1 block is neutralized too: a raw NEL (U+0085) would render as a line - // break — forging a second audit entry — and U+009B is a CSI introducer. - // Mutation check: reverting the strip to C0/DEL only lets NEL/C1 through here. expect(audit!.includes(NEL)).toBe(false); expect(audit!.includes(C1)).toBe(false); - // The Unicode line separator U+2028 (renders as a break) and the bidi RTL - // override U+202E (reorders the line) are neutralized via the shared sanitizer's - // PROMPT_UNSAFE_INVISIBLES half. Mutation check: dropping PROMPT_UNSAFE_INVISIBLES - // from sanitizeLogText lets U+2028/U+202E through here. expect(audit!.includes(LS)).toBe(false); expect(audit!.includes(RLO)).toBe(false); - // The command's embedded newline is rendered visibly (\n), not as a real break. expect(audit).toContain('\\n'); expect(audit).toContain('Slash cmd from'); expect(audit).toContain('grp-1'); @@ -542,7 +407,9 @@ describe('sendMessage', () => { disposed?: boolean; chatType?: 'c2c' | 'group'; replyMsgId?: string; + replyMsgIdTimestamp?: number; tokenExpiresAt?: number; + accessToken?: string; }): QQChannelInstance { const ch = new QQChannel( 'test-bot', @@ -561,10 +428,8 @@ describe('sendMessage', () => { {} as unknown as ChannelAgentBridge, ); - // Set internal state for sendMessage preconditions. - // accessToken and tokenExpiresAt bypass the fetchToken flow. const chp = ch as unknown as Record; - chp['accessToken'] = 'test-token'; + chp['accessToken'] = overrides?.accessToken ?? 'test-token'; chp['tokenExpiresAt'] = overrides?.tokenExpiresAt ?? Date.now() + 3600_000; if (overrides?.disposed) chp['disposed'] = true; @@ -575,10 +440,12 @@ describe('sendMessage', () => { ); } if (overrides?.replyMsgId) { - (chp['replyMsgId'] as Map).set( - 'test-chat-id', - overrides.replyMsgId, - ); + ( + chp['replyMsgId'] as Map + ).set('test-chat-id', { + msgId: overrides.replyMsgId, + timestamp: overrides.replyMsgIdTimestamp ?? Date.now(), + }); } return ch; @@ -594,7 +461,7 @@ describe('sendMessage', () => { mockFetchGatewayUrl.mockResolvedValue('wss://gateway.qq.test/ws'); }); - it('sends plain text to C2C chat with msg_type=0', async () => { + it('sends markdown-first (msg_type=2) for plain text', async () => { const ch = makeChannel({ chatType: 'c2c' }); await ch.sendMessage('test-chat-id', 'hello'); @@ -603,7 +470,7 @@ describe('sendMessage', () => { 'https://api.sgroup.qq.com', '/v2/users/test-chat-id/messages', 'test-token', - { content: 'hello', msg_type: 0 }, + { msg_type: 2, markdown: { content: 'hello' } }, ); }); @@ -628,11 +495,11 @@ describe('sendMessage', () => { 'https://api.sgroup.qq.com', '/v2/groups/test-chat-id/messages', 'test-token', - { content: 'hello', msg_type: 0 }, + { msg_type: 2, markdown: { content: 'hello' } }, ); }); - it('falls back to plain text when markdown is rejected', async () => { + it('falls back to plain text when markdown is rejected (no msgId)', async () => { const ch = makeChannel({ chatType: 'c2c' }); mockSendQQMessage .mockResolvedValueOnce(mockResponse(false, 400, 'markdown unsupported')) @@ -641,7 +508,6 @@ describe('sendMessage', () => { await ch.sendMessage('test-chat-id', '**bold**'); expect(mockSendQQMessage).toHaveBeenCalledTimes(2); - // First attempt: markdown expect(mockSendQQMessage).toHaveBeenNthCalledWith( 1, 'https://api.sgroup.qq.com', @@ -649,7 +515,6 @@ describe('sendMessage', () => { 'test-token', { msg_type: 2, markdown: { content: '**bold**' } }, ); - // Fallback: plain text expect(mockSendQQMessage).toHaveBeenNthCalledWith( 2, 'https://api.sgroup.qq.com', @@ -659,14 +524,59 @@ describe('sendMessage', () => { ); }); - it('stops on first chunk failure (no fallback for plain text)', async () => { + it('retries as active message when markdown passive reply is rejected', async () => { + const ch = makeChannel({ chatType: 'c2c', replyMsgId: 'msg-001' }); + mockSendQQMessage + .mockResolvedValueOnce(mockResponse(false, 400, 'markdown unsupported')) + .mockResolvedValueOnce(mockResponse(true)); + + await ch.sendMessage('test-chat-id', '**bold**'); + + expect(mockSendQQMessage).toHaveBeenCalledTimes(2); + expect(mockSendQQMessage).toHaveBeenNthCalledWith( + 1, + 'https://api.sgroup.qq.com', + '/v2/users/test-chat-id/messages', + 'test-token', + { + msg_type: 2, + markdown: { content: '**bold**' }, + msg_id: 'msg-001', + msg_seq: 1, + }, + ); + expect(mockSendQQMessage).toHaveBeenNthCalledWith( + 2, + 'https://api.sgroup.qq.com', + '/v2/users/test-chat-id/messages', + 'test-token', + { content: '**bold**', msg_type: 0, msg_id: 'msg-001', msg_seq: 2 }, + ); + }); + + it('does plain-text fallback when markdown fails without msgId', async () => { const ch = makeChannel({ chatType: 'c2c' }); - mockSendQQMessage.mockResolvedValue(mockResponse(false, 500)); + mockSendQQMessage + .mockResolvedValueOnce(mockResponse(false, 500)) + .mockResolvedValueOnce(mockResponse(true)); await ch.sendMessage('test-chat-id', 'hello'); - // Only one attempt — plain text doesn't retry, and we break on failure - expect(mockSendQQMessage).toHaveBeenCalledTimes(1); + expect(mockSendQQMessage).toHaveBeenCalledTimes(2); + expect(mockSendQQMessage).toHaveBeenNthCalledWith( + 1, + 'https://api.sgroup.qq.com', + '/v2/users/test-chat-id/messages', + 'test-token', + { msg_type: 2, markdown: { content: 'hello' } }, + ); + expect(mockSendQQMessage).toHaveBeenNthCalledWith( + 2, + 'https://api.sgroup.qq.com', + '/v2/users/test-chat-id/messages', + 'test-token', + { content: 'hello', msg_type: 0 }, + ); }); it('returns early when disposed', async () => { @@ -677,14 +587,14 @@ describe('sendMessage', () => { }); it('defaults to C2C path for unknown chatId', async () => { - const ch = makeChannel(); // no chatType set → not group → C2C path + const ch = makeChannel(); await ch.sendMessage('unknown-chat', 'hello'); expect(mockSendQQMessage).toHaveBeenCalledWith( 'https://api.sgroup.qq.com', '/v2/users/unknown-chat/messages', 'test-token', - { content: 'hello', msg_type: 0 }, + { msg_type: 2, markdown: { content: 'hello' } }, ); }); @@ -791,13 +701,14 @@ describe('sendMessage', () => { ch.disconnect(); }); - it('catches thrown sendQQMessage errors and stops sending', async () => { + it('re-throws when sendQQMessage throws', async () => { const ch = makeChannel({ chatType: 'c2c' }); mockSendQQMessage.mockRejectedValue(new Error('network down')); - await ch.sendMessage('test-chat-id', 'hello'); + await expect(ch.sendMessage('test-chat-id', 'hello')).rejects.toThrow( + 'network down', + ); - // No crash, and the catch+break prevents further attempts expect(mockSendQQMessage).toHaveBeenCalledTimes(1); }); @@ -809,30 +720,228 @@ describe('sendMessage', () => { 'https://api.sgroup.qq.com', '/v2/users/test-chat-id/messages', 'test-token', - { content: 'hello', msg_type: 0, msg_id: 'msg-456', msg_seq: 1 }, + { + msg_type: 2, + markdown: { content: 'hello' }, + msg_id: 'msg-456', + msg_seq: 1, + }, ); }); - it('sends multi-chunk text as separate messages with incrementing msg_seq', async () => { + it('sends single request even for long text (no splitting)', async () => { const ch = makeChannel({ chatType: 'c2c', replyMsgId: 'msg-789' }); - const text = 'a'.repeat(2500); // 2 chunks: 2000 + 500 + const text = 'a'.repeat(4500); await ch.sendMessage('test-chat-id', text); + expect(mockSendQQMessage).toHaveBeenCalledTimes(1); + expect(mockSendQQMessage).toHaveBeenCalledWith( + 'https://api.sgroup.qq.com', + '/v2/users/test-chat-id/messages', + 'test-token', + { + msg_type: 2, + markdown: { content: text }, + msg_id: 'msg-789', + msg_seq: 1, + }, + ); + }); + + it('sends without msg_id when replyMsgId is older than 5 minutes', async () => { + const ch = makeChannel({ + chatType: 'c2c', + replyMsgId: 'msg-old', + replyMsgIdTimestamp: Date.now() - 300_001, + }); + + await ch.sendMessage('test-chat-id', 'hello'); + + expect(mockSendQQMessage).toHaveBeenCalledWith( + 'https://api.sgroup.qq.com', + '/v2/users/test-chat-id/messages', + 'test-token', + { msg_type: 2, markdown: { content: 'hello' } }, + ); + const callArgs = mockSendQQMessage.mock.calls[0]; + const body = callArgs[3] as Record; + expect(body['msg_id']).toBeUndefined(); + expect(body['msg_seq']).toBeUndefined(); + }); + + it('increments msg_seq on consecutive sendMessage calls', async () => { + const ch = makeChannel({ chatType: 'c2c', replyMsgId: 'msg-999' }); + + await ch.sendMessage('test-chat-id', 'first'); + await ch.sendMessage('test-chat-id', 'second'); + expect(mockSendQQMessage).toHaveBeenCalledTimes(2); expect(mockSendQQMessage).toHaveBeenNthCalledWith( 1, 'https://api.sgroup.qq.com', '/v2/users/test-chat-id/messages', 'test-token', - { content: 'a'.repeat(2000), msg_type: 0, msg_id: 'msg-789', msg_seq: 1 }, + { + msg_type: 2, + markdown: { content: 'first' }, + msg_id: 'msg-999', + msg_seq: 1, + }, ); expect(mockSendQQMessage).toHaveBeenNthCalledWith( 2, 'https://api.sgroup.qq.com', '/v2/users/test-chat-id/messages', 'test-token', - { content: 'a'.repeat(500), msg_type: 0, msg_id: 'msg-789', msg_seq: 2 }, + { + msg_type: 2, + markdown: { content: 'second' }, + msg_id: 'msg-999', + msg_seq: 2, + }, + ); + }); + + it('skips plain-text fallback when active retry fails (msgId present)', async () => { + const ch = makeChannel({ chatType: 'c2c', replyMsgId: 'msg-001' }); + mockSendQQMessage + .mockResolvedValueOnce(mockResponse(false, 400, 'markdown rejected')) + .mockResolvedValueOnce(mockResponse(false, 500, 'server error')); + + await ch.sendMessage('test-chat-id', '**bold**'); + + expect(mockSendQQMessage).toHaveBeenCalledTimes(2); + }); + + it('stops at 429 early return after active retry rate-limited', async () => { + const ch = makeChannel({ chatType: 'c2c', replyMsgId: 'msg-429' }); + mockSendQQMessage + .mockResolvedValueOnce(mockResponse(false, 400, 'markdown rejected')) + .mockResolvedValueOnce(mockResponse(false, 429, 'rate limited')); + + await ch.sendMessage('test-chat-id', '**bold**'); + + expect(mockSendQQMessage).toHaveBeenCalledTimes(2); + const secondBody = mockSendQQMessage.mock.calls[1][3] as Record< + string, + unknown + >; + expect(secondBody['msg_type']).toBe(0); + }); + + it('rolls back msgSeqMap when sendQQMessage throws with replyMsgId set', async () => { + const ch = makeChannel({ chatType: 'c2c' }); + const chp = ch as unknown as Record; + ( + chp['replyMsgId'] as Map + ).set('test-chat-id', { + msgId: 'msg-rollback', + timestamp: Date.now(), + }); + + const msgSeqMap = chp['msgSeqMap'] as Map; + msgSeqMap.set('msg-rollback', 5); + + mockSendQQMessage.mockRejectedValue(new Error('connection reset')); + + await expect(ch.sendMessage('test-chat-id', 'hello')).rejects.toThrow( + 'connection reset', ); + + expect(msgSeqMap.get('msg-rollback')).toBe(5); + }); + + it('rolls back msgSeqMap when sendQQMessage throws with replyMsgId (new session)', async () => { + const ch = makeChannel({ chatType: 'c2c' }); + const chp = ch as unknown as Record; + ( + chp['replyMsgId'] as Map + ).set('test-chat-id', { + msgId: 'msg-new', + timestamp: Date.now(), + }); + + const msgSeqMap = chp['msgSeqMap'] as Map; + + mockSendQQMessage.mockRejectedValue(new Error('network error')); + + await expect(ch.sendMessage('test-chat-id', 'hello')).rejects.toThrow( + 'network error', + ); + + expect(msgSeqMap.get('msg-new')).toBe(0); + }); +}); + +describe('setReplyMsgId', () => { + function makeChannel(): QQChannelInstance { + const ch = new QQChannel( + 'test-bot', + { + type: 'qq', + token: '', + senderPolicy: 'open' as const, + allowedUsers: [], + sessionScope: 'user' as const, + cwd: '/tmp', + groupPolicy: 'disabled' as const, + groups: {}, + appID: 'test-app-id', + appSecret: 'test-secret', + }, + {} as unknown as ChannelAgentBridge, + ); + const chp = ch as unknown as Record; + chp['accessToken'] = 'test-token'; + chp['tokenExpiresAt'] = Date.now() + 3600_000; + return ch; + } + + it('cleans up old msgSeqMap entry when setting new replyMsgId for same chatId', () => { + const ch = makeChannel(); + const chp = ch as unknown as Record; + + const replyMsgId = chp['replyMsgId'] as Map< + string, + { msgId: string; timestamp: number } + >; + const msgSeqMap = chp['msgSeqMap'] as Map; + + replyMsgId.set('test-chat-id', { + msgId: 'old-msg-id', + timestamp: Date.now(), + }); + msgSeqMap.set('old-msg-id', 5); + msgSeqMap.set('other-msg-id', 10); + + (chp['setReplyMsgId'] as (chatId: string, msgId: string) => void)( + 'test-chat-id', + 'new-msg-id', + ); + + expect(msgSeqMap.has('old-msg-id')).toBe(false); + expect(msgSeqMap.get('other-msg-id')).toBe(10); + expect(replyMsgId.get('test-chat-id')!.msgId).toBe('new-msg-id'); + }); + + it('does nothing when chatId has no prior replyMsgId', () => { + const ch = makeChannel(); + const chp = ch as unknown as Record; + + const replyMsgId = chp['replyMsgId'] as Map< + string, + { msgId: string; timestamp: number } + >; + const msgSeqMap = chp['msgSeqMap'] as Map; + msgSeqMap.set('existing-seq', 3); + + (chp['setReplyMsgId'] as (chatId: string, msgId: string) => void)( + 'new-chat', + 'msg-new', + ); + + expect(msgSeqMap.get('existing-seq')).toBe(3); + expect(replyMsgId.get('new-chat')!.msgId).toBe('msg-new'); }); }); From ca70d6ef07ab801aa0190d870cc5e042cc7cb369 Mon Sep 17 00:00:00 2001 From: Eric-GoodBoy-Tech <634718930@qq.com> Date: Fri, 3 Jul 2026 07:11:48 +0800 Subject: [PATCH 02/10] =?UTF-8?q?fix(qqbot):=20address=20review=20feedback?= =?UTF-8?q?=20=E2=80=94=20seq=20gap,=20429=20short-circuit,=20state=20pers?= =?UTF-8?q?istence,=20input=20validation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix msg_seq gap on active retry: rollback to nextSeq-1 sends nextSeq, not nextSeq+1 - Add race guard: check replyMsgId still current before updating msgSeqMap on success - Short-circuit on 429 early: bail after markdown 429 instead of retrying - Log MESSAGE DROPPED and persist state when both passive+active send fail - Drain response body on plain-text fallback to prevent socket leak - Persist after cleanup timer eviction (saveQQState) - Validate msgSeqMap entries as [string, number] in restoreQQState - Add Number.isFinite guard for timestamp in restoreQQState - Eagerly delete expired replyMsgId entries on first TTL check - Remove redundant saveQQState() calls after setReplyMsgId (handles itself) - Fix instruction string: remove stale auto-chunk mention (splitText removed) - Fix misleading log: expired reply says 'without msg_id' not 'active message' --- packages/channels/qqbot/src/QQChannel.ts | 42 +++++++++++++++++++----- packages/channels/qqbot/src/send.test.ts | 15 +++++---- 2 files changed, 42 insertions(+), 15 deletions(-) diff --git a/packages/channels/qqbot/src/QQChannel.ts b/packages/channels/qqbot/src/QQChannel.ts index 2b5c3a6852a..bc52e8b70c0 100644 --- a/packages/channels/qqbot/src/QQChannel.ts +++ b/packages/channels/qqbot/src/QQChannel.ts @@ -157,7 +157,7 @@ export class QQChannel extends ChannelBase { '## QQ Bot Channel', '', '你是通过 QQ Bot 与用户对话的 AI 助手。', - '回复控制在 2000 字符以内(超长会自动分块),支持 Markdown 格式。', + '回复控制在 2000 字符以内,支持 Markdown 格式。', ].join('\n'); } for (let attempt = 0; attempt < 3; attempt++) { @@ -212,8 +212,10 @@ export class QQChannel extends ChannelBase { entry && Date.now() - entry.timestamp < 300_000 ? entry.msgId : undefined; if (entry && !msgId) { process.stderr.write( - `[QQ:${this.name}] replyMsgId entry expired for ${sanitizeLogText(chatId, 64)}, falling back to active message\n`, + `[QQ:${this.name}] replyMsgId entry expired for ${sanitizeLogText(chatId, 64)}, reply context expired, sending without msg_id\n`, ); + this.msgSeqMap.delete(entry.msgId); + this.replyMsgId.delete(chatId); } let nextSeq = 0; @@ -244,6 +246,15 @@ export class QQChannel extends ChannelBase { `[QQ:${this.name}] Markdown rejected (HTTP ${resp.status}: ${sanitizeLogText(errBody, 200)})\n`, ); + // 429 = rate-limited — do not retry, bail immediately + if (resp.status === 429) { + process.stderr.write( + `[QQ:${this.name}] MESSAGE DROPPED: rate-limited (429) on markdown attempt for ${sanitizeLogText(chatId, 64)}\n`, + ); + if (msgId) this.saveQQState(); + return; + } + if (msgId) { this.msgSeqMap.set(msgId, nextSeq - 1); rollbackApplied = true; @@ -251,7 +262,7 @@ export class QQChannel extends ChannelBase { content: text, msg_type: 0, msg_id: msgId, - msg_seq: nextSeq + 1, + msg_seq: nextSeq, }; const activeResp = await sendQQMessage( route.base, @@ -260,7 +271,10 @@ export class QQChannel extends ChannelBase { activeBody, ); if (activeResp.ok) { - this.msgSeqMap.set(msgId, nextSeq + 1); + const current = this.replyMsgId.get(chatId); + if (current?.msgId === msgId) { + this.msgSeqMap.set(msgId, nextSeq); + } this.saveQQState(); return; } @@ -276,8 +290,16 @@ export class QQChannel extends ChannelBase { } } - // Active retry failed — skip plain-text fallback for passive replies - if (msgId) return; + // Passive reply: both markdown and active retry failed + if (msgId) { + if (resp.status !== 429) { + process.stderr.write( + `[QQ:${this.name}] MESSAGE DROPPED: both passive and active send failed for ${sanitizeLogText(chatId, 64)}\n`, + ); + } + this.saveQQState(); + return; + } // Plain-text fallback for active messages (no reply context) const plainBody: Record = { @@ -291,6 +313,7 @@ export class QQChannel extends ChannelBase { plainBody, ); if (!fallbackRes.ok) { + await fallbackRes.text().catch(() => ''); process.stderr.write( `[QQ:${this.name}] Plain-text fallback failed: ${fallbackRes.status}\n`, ); @@ -508,7 +531,9 @@ export class QQChannel extends ChannelBase { const entry = v as { msgId?: unknown; timestamp?: unknown }; return ( typeof entry.msgId === 'string' && - typeof entry.timestamp === 'number' + entry.msgId.length <= 128 && + typeof entry.timestamp === 'number' && + Number.isFinite(entry.timestamp) ); }), ) as Map; @@ -653,6 +678,7 @@ export class QQChannel extends ChannelBase { this.replyMsgId.delete(chatId); } } + this.saveQQState(); }, 60_000); this.replyMsgIdCleanupTimer.unref(); } @@ -1099,7 +1125,6 @@ export class QQChannel extends ChannelBase { const chatId = event.author.user_openid || event.author.id; this.chatTypeMap.set(chatId, 'c2c'); this.setReplyMsgId(chatId, event.id); - this.saveQQState(); this.handleInbound({ channelName: this.name, senderId: chatId, @@ -1128,7 +1153,6 @@ export class QQChannel extends ChannelBase { const chatId = event.group_openid; this.chatTypeMap.set(chatId, 'group'); this.setReplyMsgId(chatId, event.id); - this.saveQQState(); const senderName = event.author.username || event.author.id || 'QQ User'; // Strip @mention tags from message content. QQ Bot API docs state the API // cleans these, but the format varies across API versions: diff --git a/packages/channels/qqbot/src/send.test.ts b/packages/channels/qqbot/src/send.test.ts index db8c7e0cc7a..c6ee602592e 100644 --- a/packages/channels/qqbot/src/send.test.ts +++ b/packages/channels/qqbot/src/send.test.ts @@ -550,7 +550,7 @@ describe('sendMessage', () => { 'https://api.sgroup.qq.com', '/v2/users/test-chat-id/messages', 'test-token', - { content: '**bold**', msg_type: 0, msg_id: 'msg-001', msg_seq: 2 }, + { content: '**bold**', msg_type: 0, msg_id: 'msg-001', msg_seq: 1 }, ); }); @@ -1216,12 +1216,15 @@ describe('restoreQQState validation filters', () => { const ch = makeChannel(); (ch as unknown as { restoreQQState: () => boolean }).restoreQQState(); - const replyMsgId = (ch as unknown as { replyMsgId: Map }) - .replyMsgId; + const replyMsgId = ( + ch as unknown as { + replyMsgId: Map; + } + ).replyMsgId; expect(replyMsgId.size).toBe(3); - expect(replyMsgId.get('a')).toBe('valid-id'); - expect(replyMsgId.get('b')).toBe('x'.repeat(128)); - expect(replyMsgId.get('f')).toBe(''); + expect(replyMsgId.get('a')?.msgId).toBe('valid-id'); + expect(replyMsgId.get('b')?.msgId).toBe('x'.repeat(128)); + expect(replyMsgId.get('f')?.msgId).toBe(''); expect(replyMsgId.has('c')).toBe(false); expect(replyMsgId.has('d')).toBe(false); expect(replyMsgId.has('e')).toBe(false); From ee30adc72137f7c4622654ae1a0ad50f69c4b381 Mon Sep 17 00:00:00 2001 From: Eric-GoodBoy-Tech <634718930@qq.com> Date: Fri, 3 Jul 2026 16:46:21 +0800 Subject: [PATCH 03/10] fix(qqbot): align sendMessage fallback and replyMsgId cleanup with feat branch --- packages/channels/qqbot/src/QQChannel.ts | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/packages/channels/qqbot/src/QQChannel.ts b/packages/channels/qqbot/src/QQChannel.ts index bc52e8b70c0..a31c20cd95e 100644 --- a/packages/channels/qqbot/src/QQChannel.ts +++ b/packages/channels/qqbot/src/QQChannel.ts @@ -290,16 +290,7 @@ export class QQChannel extends ChannelBase { } } - // Passive reply: both markdown and active retry failed - if (msgId) { - if (resp.status !== 429) { - process.stderr.write( - `[QQ:${this.name}] MESSAGE DROPPED: both passive and active send failed for ${sanitizeLogText(chatId, 64)}\n`, - ); - } - this.saveQQState(); - return; - } + if (msgId) return; // Plain-text fallback for active messages (no reply context) const plainBody: Record = { @@ -678,7 +669,6 @@ export class QQChannel extends ChannelBase { this.replyMsgId.delete(chatId); } } - this.saveQQState(); }, 60_000); this.replyMsgIdCleanupTimer.unref(); } From bfd0099f12ecdb9316970d2e64281e246b1b3396 Mon Sep 17 00:00:00 2001 From: Eric-GoodBoy-Tech <634718930@qq.com> Date: Fri, 3 Jul 2026 17:11:21 +0800 Subject: [PATCH 04/10] =?UTF-8?q?fix(qqbot):=20address=20PR=20#6201=20revi?= =?UTF-8?q?ew=20comments=20=E2=80=94=20setReplyMsgId=20guard,=20cleanup=20?= =?UTF-8?q?persistence,=20TTL=20constant,=20test=20coverage?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/channels/qqbot/src/QQChannel.ts | 17 ++-- packages/channels/qqbot/src/send.test.ts | 108 +++++++++++++++++++++++ 2 files changed, 120 insertions(+), 5 deletions(-) diff --git a/packages/channels/qqbot/src/QQChannel.ts b/packages/channels/qqbot/src/QQChannel.ts index a31c20cd95e..a05620b8bf1 100644 --- a/packages/channels/qqbot/src/QQChannel.ts +++ b/packages/channels/qqbot/src/QQChannel.ts @@ -104,6 +104,8 @@ export class QQChannel extends ChannelBase { private msgSeqMap: Map = new Map(); /** Periodic cleanup timer for expired replyMsgId entries. */ private replyMsgIdCleanupTimer: ReturnType | null = null; + /** 5-minute TTL for replyMsgId entries and seenMessages dedup. */ + private static readonly REPLY_MSG_ID_TTL_MS = 300_000; /** Path to persisted QQ routing state: chatTypeMap, replyMsgId, msgSeqMap. */ private readonly qqStatePath: string; @@ -209,7 +211,9 @@ export class QQChannel extends ChannelBase { // Look up reply context with TTL check const entry = this.replyMsgId.get(chatId); const msgId = - entry && Date.now() - entry.timestamp < 300_000 ? entry.msgId : undefined; + entry && Date.now() - entry.timestamp < QQChannel.REPLY_MSG_ID_TTL_MS + ? entry.msgId + : undefined; if (entry && !msgId) { process.stderr.write( `[QQ:${this.name}] replyMsgId entry expired for ${sanitizeLogText(chatId, 64)}, reply context expired, sending without msg_id\n`, @@ -320,7 +324,7 @@ export class QQChannel extends ChannelBase { this.msgSeqMap.set(msgId, nextSeq - 1); this.saveQQState(); } - if (String(e).includes('429')) { + if (e instanceof Error && /429/.test(e.message)) { process.stderr.write( `[QQ:${this.name}] MESSAGE DROPPED: rate-limited (429) on all send attempts for ${sanitizeLogText(chatId, 64)}\n`, ); @@ -647,7 +651,7 @@ export class QQChannel extends ChannelBase { */ private setReplyMsgId(chatId: string, msgId: string): void { const oldEntry = this.replyMsgId.get(chatId); - if (oldEntry) { + if (oldEntry && oldEntry.msgId !== msgId) { this.msgSeqMap.delete(oldEntry.msgId); } this.replyMsgId.set(chatId, { msgId, timestamp: Date.now() }); @@ -662,13 +666,16 @@ export class QQChannel extends ChannelBase { private startReplyMsgIdCleanup(): void { this.stopReplyMsgIdCleanup(); this.replyMsgIdCleanupTimer = setInterval(() => { - const cutoff = Date.now() - 300_000; + const cutoff = Date.now() - QQChannel.REPLY_MSG_ID_TTL_MS; + let dirty = false; for (const [chatId, entry] of this.replyMsgId) { if (entry.timestamp < cutoff) { this.msgSeqMap.delete(entry.msgId); this.replyMsgId.delete(chatId); + dirty = true; } } + if (dirty) this.saveQQState(); }, 60_000); this.replyMsgIdCleanupTimer.unref(); } @@ -1091,7 +1098,7 @@ export class QQChannel extends ChannelBase { // Evict entries older than 5 minutes if (!this.seenCleanupTimer) { this.seenCleanupTimer = setInterval(() => { - const cutoff = Date.now() - 300_000; + const cutoff = Date.now() - QQChannel.REPLY_MSG_ID_TTL_MS; for (const [id, ts] of this.seenMessages) { if (ts < cutoff) this.seenMessages.delete(id); } diff --git a/packages/channels/qqbot/src/send.test.ts b/packages/channels/qqbot/src/send.test.ts index c6ee602592e..78364d84fba 100644 --- a/packages/channels/qqbot/src/send.test.ts +++ b/packages/channels/qqbot/src/send.test.ts @@ -579,6 +579,15 @@ describe('sendMessage', () => { ); }); + it('logs and returns when plain-text fallback also fails', async () => { + const ch = makeChannel({ chatType: 'c2c' }); + mockSendQQMessage + .mockResolvedValueOnce(mockResponse(false, 500)) + .mockResolvedValueOnce(mockResponse(false, 500)); + await ch.sendMessage('test-chat-id', 'hello'); + expect(mockSendQQMessage).toHaveBeenCalledTimes(2); + }); + it('returns early when disposed', async () => { const ch = makeChannel({ disposed: true, chatType: 'c2c' }); await ch.sendMessage('test-chat-id', 'hello'); @@ -767,6 +776,12 @@ describe('sendMessage', () => { const body = callArgs[3] as Record; expect(body['msg_id']).toBeUndefined(); expect(body['msg_seq']).toBeUndefined(); + // Verify expired entries were cleaned from maps + const chp = ch as unknown as Record; + const replyMap = chp['replyMsgId'] as Map; + const seqMap = chp['msgSeqMap'] as Map; + expect(replyMap.has('test-chat-id')).toBe(false); + expect(seqMap.has('msg-old')).toBe(false); }); it('increments msg_seq on consecutive sendMessage calls', async () => { @@ -1461,3 +1476,96 @@ describe('atomic state persistence', () => { vi.useRealTimers(); }); }); + +describe('replyMsgId cleanup timer', () => { + function makeChannel(): QQChannelInstance { + const ch = new QQChannel( + 'test-bot', + { + type: 'qq', + token: '', + senderPolicy: 'open' as const, + allowedUsers: [], + sessionScope: 'user' as const, + cwd: '/tmp', + groupPolicy: 'disabled' as const, + groups: {}, + appID: 'test-app-id', + appSecret: 'test-secret', + }, + {} as unknown as ChannelAgentBridge, + ); + return ch; + } + + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('evicts expired replyMsgId entries and persists cleanup', () => { + vi.useFakeTimers(); + const ch = makeChannel(); + const chp = ch as unknown as Record; + const replyMsgId = chp['replyMsgId'] as Map< + string, + { msgId: string; timestamp: number } + >; + const msgSeqMap = chp['msgSeqMap'] as Map; + const saveSpy = vi.spyOn(chp as { saveQQState: () => void }, 'saveQQState'); + + // Seed expired entry (6 min old) + replyMsgId.set('chat-old', { + msgId: 'msg-old', + timestamp: Date.now() - 360_000, + }); + msgSeqMap.set('msg-old', 5); + // Seed fresh entry (1 min old) + replyMsgId.set('chat-fresh', { + msgId: 'msg-fresh', + timestamp: Date.now() - 60_000, + }); + msgSeqMap.set('msg-fresh', 2); + + (chp['startReplyMsgIdCleanup'] as () => void).call(ch); + vi.advanceTimersByTime(60_000); + + // Expired entries removed + expect(replyMsgId.has('chat-old')).toBe(false); + expect(msgSeqMap.has('msg-old')).toBe(false); + // Fresh entries remain + expect(replyMsgId.has('chat-fresh')).toBe(true); + expect(replyMsgId.get('chat-fresh')!.msgId).toBe('msg-fresh'); + expect(msgSeqMap.get('msg-fresh')).toBe(2); + // Cleanup was persisted + expect(saveSpy).toHaveBeenCalledTimes(1); + + saveSpy.mockRestore(); + ch.disconnect(); + }); + + it('does not call saveQQState when no entries are evicted', () => { + vi.useFakeTimers(); + const ch = makeChannel(); + const chp = ch as unknown as Record; + const replyMsgId = chp['replyMsgId'] as Map< + string, + { msgId: string; timestamp: number } + >; + const saveSpy = vi.spyOn(chp as { saveQQState: () => void }, 'saveQQState'); + + // Only fresh entries + replyMsgId.set('chat-fresh', { + msgId: 'msg-fresh', + timestamp: Date.now() - 60_000, + }); + + (chp['startReplyMsgIdCleanup'] as () => void).call(ch); + vi.advanceTimersByTime(60_000); + + expect(replyMsgId.has('chat-fresh')).toBe(true); + expect(saveSpy).not.toHaveBeenCalled(); + + saveSpy.mockRestore(); + ch.disconnect(); + }); +}); From 3b67b0261befa9acbc2339961aca3aac7cac5c0d Mon Sep 17 00:00:00 2001 From: Eric-GoodBoy-Tech <634718930@qq.com> Date: Fri, 3 Jul 2026 18:03:04 +0800 Subject: [PATCH 05/10] fix(qqbot): address PR #6201 review round 2 --- packages/channels/qqbot/src/QQChannel.ts | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/packages/channels/qqbot/src/QQChannel.ts b/packages/channels/qqbot/src/QQChannel.ts index a05620b8bf1..34e7922cb2d 100644 --- a/packages/channels/qqbot/src/QQChannel.ts +++ b/packages/channels/qqbot/src/QQChannel.ts @@ -247,7 +247,7 @@ export class QQChannel extends ChannelBase { if (!resp.ok) { const errBody = await resp.text().catch(() => ''); process.stderr.write( - `[QQ:${this.name}] Markdown rejected (HTTP ${resp.status}: ${sanitizeLogText(errBody, 200)})\n`, + `[QQ:${this.name}] Send failed (HTTP ${resp.status}: ${sanitizeLogText(errBody, 200)})\n`, ); // 429 = rate-limited — do not retry, bail immediately @@ -292,10 +292,14 @@ export class QQChannel extends ChannelBase { this.saveQQState(); return; } + // Active retry failed with non-429 — don't fall through to plain-text + process.stderr.write( + `[QQ:${this.name}] MESSAGE DROPPED: both passive and active send failed for ${sanitizeLogText(chatId, 64)}\n`, + ); + this.saveQQState(); + return; } - if (msgId) return; - // Plain-text fallback for active messages (no reply context) const plainBody: Record = { content: text, @@ -309,6 +313,12 @@ export class QQChannel extends ChannelBase { ); if (!fallbackRes.ok) { await fallbackRes.text().catch(() => ''); + if (fallbackRes.status === 429) { + process.stderr.write( + `[QQ:${this.name}] MESSAGE DROPPED: rate-limited (429) on plain-text fallback for ${sanitizeLogText(chatId, 64)}\n`, + ); + return; + } process.stderr.write( `[QQ:${this.name}] Plain-text fallback failed: ${fallbackRes.status}\n`, ); @@ -324,11 +334,8 @@ export class QQChannel extends ChannelBase { this.msgSeqMap.set(msgId, nextSeq - 1); this.saveQQState(); } - if (e instanceof Error && /429/.test(e.message)) { - process.stderr.write( - `[QQ:${this.name}] MESSAGE DROPPED: rate-limited (429) on all send attempts for ${sanitizeLogText(chatId, 64)}\n`, - ); - } + // Note: sendQQMessage only throws on network/timeout errors, never HTTP status. + // Rate-limit (429) handling is in the resp.status checks above. process.stderr.write( `[QQ:${this.name}] Send error: ${sanitizeLogText(e instanceof Error ? e.message : String(e), 200)}\n`, ); From 52c1380f6730a37ab46fb592f90353bb8423e60d Mon Sep 17 00:00:00 2001 From: Eric-GoodBoy-Tech <634718930@qq.com> Date: Fri, 3 Jul 2026 18:53:34 +0800 Subject: [PATCH 06/10] =?UTF-8?q?fix(qqbot):=20address=20PR=20#6201=20revi?= =?UTF-8?q?ew=20round=203=20=E2=80=94=20add=20MESSAGE=20DROPPED=20prefix?= =?UTF-8?q?=20to=20plain-text=20fallback=20error=20log?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/channels/qqbot/src/QQChannel.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/channels/qqbot/src/QQChannel.ts b/packages/channels/qqbot/src/QQChannel.ts index 34e7922cb2d..55a99ceb149 100644 --- a/packages/channels/qqbot/src/QQChannel.ts +++ b/packages/channels/qqbot/src/QQChannel.ts @@ -320,7 +320,7 @@ export class QQChannel extends ChannelBase { return; } process.stderr.write( - `[QQ:${this.name}] Plain-text fallback failed: ${fallbackRes.status}\n`, + `[QQ:${this.name}] MESSAGE DROPPED: plain-text fallback failed (HTTP ${fallbackRes.status}) for ${sanitizeLogText(chatId, 64)}\n`, ); } return; From 1e5caf5e39ea0ebc3bbac9b194dd720e87e2d4a8 Mon Sep 17 00:00:00 2001 From: Eric-GoodBoy-Tech <634718930@qq.com> Date: Fri, 3 Jul 2026 19:32:58 +0800 Subject: [PATCH 07/10] fix(qqbot): address PR #6201 review round 4 - Fix catch block: always call saveQQState() regardless of rollbackApplied - Plain-text fallback log now includes error body text - Add success logs for active retry and plain-text fallback paths - Add .unref() to seenCleanupTimer for clean process exit - Add threat-model comment to group sender-name sanitization tests - Add saveQQState spy assertions to rollback tests --- packages/channels/qqbot/src/QQChannel.ts | 15 +++++++++++---- packages/channels/qqbot/src/send.test.ts | 21 +++++++++++++++++++++ 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/packages/channels/qqbot/src/QQChannel.ts b/packages/channels/qqbot/src/QQChannel.ts index 55a99ceb149..033304fa23b 100644 --- a/packages/channels/qqbot/src/QQChannel.ts +++ b/packages/channels/qqbot/src/QQChannel.ts @@ -275,6 +275,9 @@ export class QQChannel extends ChannelBase { activeBody, ); if (activeResp.ok) { + process.stderr.write( + `[QQ:${this.name}] Active retry succeeded for ${sanitizeLogText(chatId, 64)}\n`, + ); const current = this.replyMsgId.get(chatId); if (current?.msgId === msgId) { this.msgSeqMap.set(msgId, nextSeq); @@ -312,7 +315,7 @@ export class QQChannel extends ChannelBase { plainBody, ); if (!fallbackRes.ok) { - await fallbackRes.text().catch(() => ''); + const fbErrBody = await fallbackRes.text().catch(() => ''); if (fallbackRes.status === 429) { process.stderr.write( `[QQ:${this.name}] MESSAGE DROPPED: rate-limited (429) on plain-text fallback for ${sanitizeLogText(chatId, 64)}\n`, @@ -320,9 +323,13 @@ export class QQChannel extends ChannelBase { return; } process.stderr.write( - `[QQ:${this.name}] MESSAGE DROPPED: plain-text fallback failed (HTTP ${fallbackRes.status}) for ${sanitizeLogText(chatId, 64)}\n`, + `[QQ:${this.name}] MESSAGE DROPPED: plain-text fallback failed (HTTP ${fallbackRes.status}: ${sanitizeLogText(fbErrBody, 200)}) for ${sanitizeLogText(chatId, 64)}\n`, ); + return; } + process.stderr.write( + `[QQ:${this.name}] Plain-text fallback succeeded for ${sanitizeLogText(chatId, 64)}\n`, + ); return; } @@ -332,8 +339,8 @@ export class QQChannel extends ChannelBase { // Rollback on failure if we haven't already if (msgId && !rollbackApplied) { this.msgSeqMap.set(msgId, nextSeq - 1); - this.saveQQState(); } + if (msgId) this.saveQQState(); // Note: sendQQMessage only throws on network/timeout errors, never HTTP status. // Rate-limit (429) handling is in the resp.status checks above. process.stderr.write( @@ -1113,7 +1120,7 @@ export class QQChannel extends ChannelBase { clearInterval(this.seenCleanupTimer!); this.seenCleanupTimer = null; } - }, 60_000); + }, 60_000).unref(); } return false; } diff --git a/packages/channels/qqbot/src/send.test.ts b/packages/channels/qqbot/src/send.test.ts index 78364d84fba..804619fadbe 100644 --- a/packages/channels/qqbot/src/send.test.ts +++ b/packages/channels/qqbot/src/send.test.ts @@ -256,6 +256,22 @@ describe('session persistence paths', () => { }); }); +// Security model for group sender-name sanitization: +// QQ group message authors supply their own nickname (username), which is +// attacker-controlled. The channel prepends `[sanitizeLogText(name, 64)]:` +// before each message body. An unsanitized name could contain: +// +// - Newlines or ANSI escapes → forge fake audit entries in handleGroup logs +// - Unicode line breaks (NEL U+0085, C1 U+009B, LS U+2028) → bypass regex- +// based line-splitting, creating a second visual line in audit output +// - BiDi overrides (RLO U+202E) → reverse text in terminals that render it +// - Brackets `[]` → prematurely close the [name] tag so attacker content +// appears outside the bracket, impersonating a system message +// +// `sanitizeLogText` (imported via vi.importActual so a trojan-source +// regression is caught) neutralizes: escape sequences, control chars, +// newlines, Unicode line separators, and BiDi overrides. The tests below +// validate each threat class individually. describe('group sender-name sanitization', () => { function makeChannel() { return new QQChannel( @@ -878,6 +894,10 @@ describe('sendMessage', () => { const msgSeqMap = chp['msgSeqMap'] as Map; + const saveSpy = vi.spyOn( + ch as unknown as { saveQQState: () => void }, + 'saveQQState', + ); mockSendQQMessage.mockRejectedValue(new Error('network error')); await expect(ch.sendMessage('test-chat-id', 'hello')).rejects.toThrow( @@ -885,6 +905,7 @@ describe('sendMessage', () => { ); expect(msgSeqMap.get('msg-new')).toBe(0); + expect(saveSpy).toHaveBeenCalled(); }); }); From 142683ee65574e9604e26faf5d86077881e58d08 Mon Sep 17 00:00:00 2001 From: Eric-GoodBoy-Tech <634718930@qq.com> Date: Fri, 3 Jul 2026 20:06:15 +0800 Subject: [PATCH 08/10] fix(qqbot): address PR #6201 review round 5 --- packages/channels/qqbot/src/QQChannel.ts | 6 +- packages/channels/qqbot/src/send.test.ts | 85 ++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 1 deletion(-) diff --git a/packages/channels/qqbot/src/QQChannel.ts b/packages/channels/qqbot/src/QQChannel.ts index 033304fa23b..fd517fd0370 100644 --- a/packages/channels/qqbot/src/QQChannel.ts +++ b/packages/channels/qqbot/src/QQChannel.ts @@ -220,6 +220,7 @@ export class QQChannel extends ChannelBase { ); this.msgSeqMap.delete(entry.msgId); this.replyMsgId.delete(chatId); + this.saveQQState(); } let nextSeq = 0; @@ -255,7 +256,10 @@ export class QQChannel extends ChannelBase { process.stderr.write( `[QQ:${this.name}] MESSAGE DROPPED: rate-limited (429) on markdown attempt for ${sanitizeLogText(chatId, 64)}\n`, ); - if (msgId) this.saveQQState(); + if (msgId) { + this.msgSeqMap.set(msgId, nextSeq - 1); + this.saveQQState(); + } return; } diff --git a/packages/channels/qqbot/src/send.test.ts b/packages/channels/qqbot/src/send.test.ts index 804619fadbe..ccdfff8d57c 100644 --- a/packages/channels/qqbot/src/send.test.ts +++ b/packages/channels/qqbot/src/send.test.ts @@ -542,6 +542,12 @@ describe('sendMessage', () => { it('retries as active message when markdown passive reply is rejected', async () => { const ch = makeChannel({ chatType: 'c2c', replyMsgId: 'msg-001' }); + const chp = ch as unknown as Record; + const msgSeqMap = chp['msgSeqMap'] as Map; + msgSeqMap.set('msg-001', 0); + + const saveSpy = vi.spyOn(chp as { saveQQState: () => void }, 'saveQQState'); + mockSendQQMessage .mockResolvedValueOnce(mockResponse(false, 400, 'markdown unsupported')) .mockResolvedValueOnce(mockResponse(true)); @@ -568,6 +574,13 @@ describe('sendMessage', () => { 'test-token', { content: '**bold**', msg_type: 0, msg_id: 'msg-001', msg_seq: 1 }, ); + + // Post-success state: sequence reflects successful send (not rolled back) + expect(msgSeqMap.get('msg-001')).toBe(1); + // saveQQState was called to persist + expect(saveSpy).toHaveBeenCalled(); + + saveSpy.mockRestore(); }); it('does plain-text fallback when markdown fails without msgId', async () => { @@ -907,6 +920,78 @@ describe('sendMessage', () => { expect(msgSeqMap.get('msg-new')).toBe(0); expect(saveSpy).toHaveBeenCalled(); }); + it('returns early without sending when text is ', async () => { + const ch = makeChannel({ chatType: 'c2c' }); + await ch.sendMessage('test-chat-id', ''); + + expect(mockSendQQMessage).not.toHaveBeenCalled(); + }); + + it('returns early for with leading/trailing whitespace', async () => { + const ch = makeChannel({ chatType: 'c2c' }); + await ch.sendMessage('test-chat-id', ' '); + + expect(mockSendQQMessage).not.toHaveBeenCalled(); + }); + + it('writes stderr when is suppressed', async () => { + const ch = makeChannel({ chatType: 'c2c' }); + const writes: string[] = []; + const spy = vi + .spyOn(process.stderr, 'write') + .mockImplementation((chunk: unknown) => { + writes.push(String(chunk)); + return true; + }); + + await ch.sendMessage('test-chat-id', ''); + + spy.mockRestore(); + expect(writes.some((w) => w.includes(' skipped'))).toBe(true); + }); + + it('does not mutate msgSeqMap or replyMsgId on suppression', async () => { + const ch = makeChannel({ chatType: 'c2c', replyMsgId: 'msg-001' }); + const chp = ch as unknown as Record; + const msgSeqMap = chp['msgSeqMap'] as Map; + msgSeqMap.set('msg-001', 3); + + await ch.sendMessage('test-chat-id', ''); + + // msgSeqMap should be unchanged + expect(msgSeqMap.get('msg-001')).toBe(3); + // replyMsgId should still be present + const replyMsgId = chp['replyMsgId'] as Map; + expect(replyMsgId.has('test-chat-id')).toBe(true); + expect(mockSendQQMessage).not.toHaveBeenCalled(); + }); + + it('stops at 429 on first markdown attempt and rolls back msgSeqMap', async () => { + const ch = makeChannel({ chatType: 'c2c', replyMsgId: 'msg-429-1st' }); + const chp = ch as unknown as Record; + const msgSeqMap = chp['msgSeqMap'] as Map; + msgSeqMap.set('msg-429-1st', 0); + + const saveSpy = vi.spyOn(chp as { saveQQState: () => void }, 'saveQQState'); + + mockSendQQMessage.mockResolvedValueOnce(mockResponse(false, 429)); + + await ch.sendMessage('test-chat-id', '**bold**'); + + // No second call — 429 bails immediately + expect(mockSendQQMessage).toHaveBeenCalledTimes(1); + // First call had msg_seq=1 (0 + 1) + const body = mockSendQQMessage.mock.calls[0][3] as Record; + expect(body['msg_seq']).toBe(1); + expect(body['msg_id']).toBe('msg-429-1st'); + + // msgSeqMap rolled back from 1 to 0 + expect(msgSeqMap.get('msg-429-1st')).toBe(0); + // saveQQState was called to persist the rollback + expect(saveSpy).toHaveBeenCalled(); + + saveSpy.mockRestore(); + }); }); describe('setReplyMsgId', () => { From 35ed77c5f1e5d3418ccbc98653481466c4f03f74 Mon Sep 17 00:00:00 2001 From: Eric-GoodBoy-Tech <634718930@qq.com> Date: Fri, 3 Jul 2026 20:44:25 +0800 Subject: [PATCH 09/10] fix(qqbot): address PR #6201 review round 6 --- packages/channels/qqbot/src/QQChannel.ts | 6 ++++-- packages/channels/qqbot/src/send.test.ts | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/packages/channels/qqbot/src/QQChannel.ts b/packages/channels/qqbot/src/QQChannel.ts index fd517fd0370..ed94f2337e6 100644 --- a/packages/channels/qqbot/src/QQChannel.ts +++ b/packages/channels/qqbot/src/QQChannel.ts @@ -287,6 +287,7 @@ export class QQChannel extends ChannelBase { this.msgSeqMap.set(msgId, nextSeq); } this.saveQQState(); + await activeResp.text().catch(() => ''); return; } process.stderr.write( @@ -294,7 +295,7 @@ export class QQChannel extends ChannelBase { ); if (activeResp.status === 429) { process.stderr.write( - `[QQ:${this.name}] Active retry rate-limited (HTTP 429), giving up\n`, + `[QQ:${this.name}] MESSAGE DROPPED: active retry rate-limited (HTTP 429) for ${sanitizeLogText(chatId, 64)}\n`, ); this.saveQQState(); return; @@ -334,10 +335,11 @@ export class QQChannel extends ChannelBase { process.stderr.write( `[QQ:${this.name}] Plain-text fallback succeeded for ${sanitizeLogText(chatId, 64)}\n`, ); + await fallbackRes.text().catch(() => ''); return; } - // Success — persist msgSeq + await resp.text().catch(() => ''); if (msgId) this.saveQQState(); } catch (e) { // Rollback on failure if we haven't already diff --git a/packages/channels/qqbot/src/send.test.ts b/packages/channels/qqbot/src/send.test.ts index ccdfff8d57c..ef63f9556f1 100644 --- a/packages/channels/qqbot/src/send.test.ts +++ b/packages/channels/qqbot/src/send.test.ts @@ -610,11 +610,24 @@ describe('sendMessage', () => { it('logs and returns when plain-text fallback also fails', async () => { const ch = makeChannel({ chatType: 'c2c' }); + const chp = ch as unknown as { saveQQState: () => void }; + const saveSpy = vi.spyOn(chp, 'saveQQState'); + const writes: string[] = []; + const stderrSpy = vi + .spyOn(process.stderr, 'write') + .mockImplementation((chunk) => { + writes.push(String(chunk)); + return true; + }); mockSendQQMessage .mockResolvedValueOnce(mockResponse(false, 500)) .mockResolvedValueOnce(mockResponse(false, 500)); await ch.sendMessage('test-chat-id', 'hello'); expect(mockSendQQMessage).toHaveBeenCalledTimes(2); + expect(writes.some((w) => w.includes('MESSAGE DROPPED'))).toBe(true); + expect(saveSpy).not.toHaveBeenCalled(); + saveSpy.mockRestore(); + stderrSpy.mockRestore(); }); it('returns early when disposed', async () => { @@ -752,6 +765,8 @@ describe('sendMessage', () => { it('includes msg_id and msg_seq when replyMsgId is set', async () => { const ch = makeChannel({ chatType: 'c2c', replyMsgId: 'msg-456' }); + const chp = ch as unknown as Record; + const saveSpy = vi.spyOn(chp as { saveQQState: () => void }, 'saveQQState'); await ch.sendMessage('test-chat-id', 'hello'); expect(mockSendQQMessage).toHaveBeenCalledWith( @@ -765,6 +780,9 @@ describe('sendMessage', () => { msg_seq: 1, }, ); + expect(saveSpy).toHaveBeenCalled(); + expect((chp['msgSeqMap'] as Map).get('msg-456')).toBe(1); + saveSpy.mockRestore(); }); it('sends single request even for long text (no splitting)', async () => { From 25d27c3f656c8bf824175faa4c058aa01d25ac34 Mon Sep 17 00:00:00 2001 From: Eric-GoodBoy-Tech <634718930@qq.com> Date: Fri, 3 Jul 2026 21:35:46 +0800 Subject: [PATCH 10/10] fix(qqbot): address PR #6201 review round 8 - Guard far-future timestamps in restoreQQState validation with an upper bound (now + REPLY_MSG_ID_TTL_MS) so corrupted state cannot pin entries permanently. - Add test for 429 without msgId returning silently (no fallback/rollback). - Add test for 429 on plain-text fallback rate-limited path. - Add test for setReplyMsgId same-msgId guard no-op branch (no delete). --- packages/channels/qqbot/src/QQChannel.ts | 3 +- packages/channels/qqbot/src/send.test.ts | 92 ++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 1 deletion(-) diff --git a/packages/channels/qqbot/src/QQChannel.ts b/packages/channels/qqbot/src/QQChannel.ts index ed94f2337e6..f1c89ca6b95 100644 --- a/packages/channels/qqbot/src/QQChannel.ts +++ b/packages/channels/qqbot/src/QQChannel.ts @@ -548,7 +548,8 @@ export class QQChannel extends ChannelBase { typeof entry.msgId === 'string' && entry.msgId.length <= 128 && typeof entry.timestamp === 'number' && - Number.isFinite(entry.timestamp) + Number.isFinite(entry.timestamp) && + entry.timestamp <= now + QQChannel.REPLY_MSG_ID_TTL_MS ); }), ) as Map; diff --git a/packages/channels/qqbot/src/send.test.ts b/packages/channels/qqbot/src/send.test.ts index ef63f9556f1..7820aa2e580 100644 --- a/packages/channels/qqbot/src/send.test.ts +++ b/packages/channels/qqbot/src/send.test.ts @@ -630,6 +630,34 @@ describe('sendMessage', () => { stderrSpy.mockRestore(); }); + it('logs MESSAGE DROPPED when plain-text fallback is rate-limited (429)', async () => { + const ch = makeChannel({ chatType: 'c2c' }); + const writes: string[] = []; + const stderrSpy = vi + .spyOn(process.stderr, 'write') + .mockImplementation((chunk) => { + writes.push(String(chunk)); + return true; + }); + + mockSendQQMessage + .mockResolvedValueOnce(mockResponse(false, 500)) + .mockResolvedValueOnce(mockResponse(false, 429)); + + await ch.sendMessage('test-chat-id', 'hello'); + + expect(mockSendQQMessage).toHaveBeenCalledTimes(2); + expect( + writes.some((w) => + w.includes( + 'MESSAGE DROPPED: rate-limited (429) on plain-text fallback', + ), + ), + ).toBe(true); + + stderrSpy.mockRestore(); + }); + it('returns early when disposed', async () => { const ch = makeChannel({ disposed: true, chatType: 'c2c' }); await ch.sendMessage('test-chat-id', 'hello'); @@ -1010,6 +1038,17 @@ describe('sendMessage', () => { saveSpy.mockRestore(); }); + + it('stops silently at 429 when no replyMsgId is set', async () => { + const ch = makeChannel({ chatType: 'c2c' }); + + mockSendQQMessage.mockResolvedValueOnce(mockResponse(false, 429)); + + await ch.sendMessage('test-chat-id', 'hello'); + + // No second call — 429 bails immediately without fallback or rollback + expect(mockSendQQMessage).toHaveBeenCalledTimes(1); + }); }); describe('setReplyMsgId', () => { @@ -1082,6 +1121,33 @@ describe('setReplyMsgId', () => { expect(msgSeqMap.get('existing-seq')).toBe(3); expect(replyMsgId.get('new-chat')!.msgId).toBe('msg-new'); }); + + it('no-ops msgSeqMap.delete when setting the same msgId for same chatId', () => { + const ch = makeChannel(); + const chp = ch as unknown as Record; + + const replyMsgId = chp['replyMsgId'] as Map< + string, + { msgId: string; timestamp: number } + >; + const msgSeqMap = chp['msgSeqMap'] as Map; + + replyMsgId.set('test-chat-id', { + msgId: 'same-msg-id', + timestamp: Date.now(), + }); + msgSeqMap.set('same-msg-id', 3); + + // Set the same msgId again — the guard should prevent delete + (chp['setReplyMsgId'] as (chatId: string, msgId: string) => void)( + 'test-chat-id', + 'same-msg-id', + ); + + // msgSeqMap entry should still exist (was not deleted) + expect(msgSeqMap.get('same-msg-id')).toBe(3); + expect(replyMsgId.get('test-chat-id')!.msgId).toBe('same-msg-id'); + }); }); describe('lifecycle status hooks', () => { @@ -1369,6 +1435,32 @@ describe('restoreQQState validation filters', () => { expect(replyMsgId.has('e')).toBe(false); }); + it('filters replyMsgId entries with far-future timestamps', () => { + vi.mocked(existsSync).mockReturnValue(true); + // Use raw JSON to force a timestamp far beyond real-time + const farFuture = Date.now() + 1e15; + vi.mocked(readFileSync).mockReturnValue( + JSON.stringify({ + replyMsgId: [ + ['a', { msgId: 'valid', timestamp: Date.now() }], + ['b', { msgId: 'far-future', timestamp: farFuture }], + ], + }), + ); + + const ch = makeChannel(); + (ch as unknown as { restoreQQState: () => boolean }).restoreQQState(); + + const replyMsgId = ( + ch as unknown as { + replyMsgId: Map; + } + ).replyMsgId; + expect(replyMsgId.size).toBe(1); + expect(replyMsgId.get('a')?.msgId).toBe('valid'); + expect(replyMsgId.has('b')).toBe(false); + }); + it('filters msgSeqMap to only accept non-negative numbers', () => { vi.mocked(existsSync).mockReturnValue(true); vi.mocked(readFileSync).mockReturnValue(