From 67da78166b4435eb82457a3f829dc6c931290ef0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9B=B9=E6=BD=87=E7=BC=A4?= <634718930@qq.com> Date: Fri, 3 Jul 2026 23:34:52 +0800 Subject: [PATCH] fix(qqbot): markdown-first send, replyMsgId TTL, and dead code removal (#6201) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 * fix(qqbot): address review feedback — seq gap, 429 short-circuit, state persistence, input validation - 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' * fix(qqbot): align sendMessage fallback and replyMsgId cleanup with feat branch * fix(qqbot): address PR #6201 review comments — setReplyMsgId guard, cleanup persistence, TTL constant, test coverage * fix(qqbot): address PR #6201 review round 2 * fix(qqbot): address PR #6201 review round 3 — add MESSAGE DROPPED prefix to plain-text fallback error log * 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 * fix(qqbot): address PR #6201 review round 5 * fix(qqbot): address PR #6201 review round 6 * 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 | 335 ++++++---- packages/channels/qqbot/src/send.test.ts | 776 ++++++++++++++++++----- 2 files changed, 822 insertions(+), 289 deletions(-) diff --git a/packages/channels/qqbot/src/QQChannel.ts b/packages/channels/qqbot/src/QQChannel.ts index b3c95d63e4..f1c89ca6b9 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,14 @@ 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; + /** 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; @@ -201,7 +159,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++) { @@ -216,6 +174,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 +197,163 @@ 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 < 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`, + ); + this.msgSeqMap.delete(entry.msgId); + this.replyMsgId.delete(chatId); + this.saveQQState(); + } - 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; - } + 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; + } - let resp = await sendQQMessage( - route.base, - route.path, - this.accessToken, - body, + const 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}] Send failed (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(() => ''); + // 429 = rate-limited — do not retry, bail immediately + if (resp.status === 429) { process.stderr.write( - `[QQ:${this.name}] Markdown rejected (HTTP ${resp.status}: ${sanitizeLogText(errBody, 200)}), retrying as plain text\n`, + `[QQ:${this.name}] MESSAGE DROPPED: rate-limited (429) on markdown attempt for ${sanitizeLogText(chatId, 64)}\n`, ); - const plainBody: Record = { - content: chunk, - msg_type: 0, - }; if (msgId) { - plainBody['msg_id'] = msgId; - plainBody['msg_seq'] = nextSeq; + this.msgSeqMap.set(msgId, nextSeq - 1); + this.saveQQState(); } - resp = await sendQQMessage( + return; + } + + if (msgId) { + this.msgSeqMap.set(msgId, nextSeq - 1); + rollbackApplied = true; + const activeBody: Record = { + content: text, + msg_type: 0, + msg_id: msgId, + msg_seq: nextSeq, + }; + const activeResp = await sendQQMessage( route.base, route.path, this.accessToken, - plainBody, + 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); + } + this.saveQQState(); + await activeResp.text().catch(() => ''); + 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}] MESSAGE DROPPED: active retry rate-limited (HTTP 429) for ${sanitizeLogText(chatId, 64)}\n`, + ); + 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 (!resp.ok) { - // Drain response body to avoid socket leak - const errBody = await resp.text().catch(() => ''); - process.stderr.write( - `[QQ:${this.name}] Send HTTP ${resp.status} (msg_seq=${body['msg_seq'] ?? '-'}): ${sanitizeLogText(errBody, 200)}\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) { - process.stderr.write( - `[QQ:${this.name}] Send error: ${sanitizeLogText(e instanceof Error ? e.message : String(e), 200)}\n`, + // 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, ); - break; + if (!fallbackRes.ok) { + 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`, + ); + return; + } + process.stderr.write( + `[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`, + ); + await fallbackRes.text().catch(() => ''); + return; } + + await resp.text().catch(() => ''); + if (msgId) this.saveQQState(); + } catch (e) { + // Rollback on failure if we haven't already + if (msgId && !rollbackApplied) { + this.msgSeqMap.set(msgId, nextSeq - 1); + } + 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( + `[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 +384,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 +529,30 @@ 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' && + entry.msgId.length <= 128 && + typeof entry.timestamp === 'number' && + Number.isFinite(entry.timestamp) && + entry.timestamp <= now + QQChannel.REPLY_MSG_ID_TTL_MS + ); + }), + ) as Map; const dropped = rawRM.length - this.replyMsgId.size; if (dropped > 0) process.stderr.write( @@ -609,6 +664,50 @@ 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 && oldEntry.msgId !== msgId) { + 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() - 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(); + } + + private stopReplyMsgIdCleanup(): void { + if (this.replyMsgIdCleanupTimer) { + clearInterval(this.replyMsgIdCleanupTimer); + this.replyMsgIdCleanupTimer = null; + } + } + // ── Token ────────────────────────────────────────────────────── private async fetchToken(): Promise { @@ -1020,7 +1119,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); } @@ -1028,7 +1127,7 @@ export class QQChannel extends ChannelBase { clearInterval(this.seenCleanupTimer!); this.seenCleanupTimer = null; } - }, 60_000); + }, 60_000).unref(); } return false; } @@ -1043,8 +1142,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.saveQQState(); + this.setReplyMsgId(chatId, event.id); this.handleInbound({ channelName: this.name, senderId: chatId, @@ -1072,8 +1170,7 @@ export class QQChannel extends ChannelBase { } const chatId = event.group_openid; this.chatTypeMap.set(chatId, 'group'); - this.replyMsgId.set(chatId, event.id); - this.saveQQState(); + this.setReplyMsgId(chatId, event.id); 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 32c15f4076..7820aa2e58 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, @@ -356,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( @@ -377,8 +293,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 +313,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 +349,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 +368,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 +388,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 +403,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 +423,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 +444,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 +456,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 +477,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 +486,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 +511,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 +524,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 +531,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 +540,122 @@ 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' }); + 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)); + + 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: 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 () => { 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('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('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 () => { @@ -677,14 +666,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,32 +780,90 @@ 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); }); 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( '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, + }, + ); + 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 () => { + const ch = makeChannel({ chatType: 'c2c', replyMsgId: 'msg-789' }); + 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 multi-chunk text as separate messages with incrementing msg_seq', async () => { - const ch = makeChannel({ chatType: 'c2c', replyMsgId: 'msg-789' }); - const text = 'a'.repeat(2500); // 2 chunks: 2000 + 500 - await ch.sendMessage('test-chat-id', text); + 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(); + // 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 () => { + 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( @@ -824,16 +871,283 @@ describe('sendMessage', () => { '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; + + 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( + 'network error', + ); + + 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(); + }); + + 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', () => { + 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'); + }); + + 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', () => { @@ -1107,17 +1421,46 @@ 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); }); + 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( @@ -1349,3 +1692,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(); + }); +});