fix(qqbot): restore per-group session isolation under thread scope - #8241
fix(qqbot): restore per-group session isolation under thread scope#8241Eric-GoodBoy-Tech wants to merge 17 commits into
Conversation
Remove the forced sessionScope='single' override for groupAllPolicy keyword/all (introduced in a3b70a5 during PR QwenLM#6457 review). It collapsed all groups, users, and DMs into one shared session, leaking context across groups and DMs. - Replace the override with a WARNING for non-thread scopes - Track reply msg_id per session so concurrent streams in one group do not cross-thread (sessionReplyMsgId anchor) - Purge orphaned :__single__ persisted routing entries on startup - Declare defaultSessionScope: 'thread' for the QQ channel - Document session isolation semantics in qqbot.md Fixes QwenLM#8238
github-actions
Bot
added
the
review/self-reported
|
Gate re-run — Aug 24, triggered by
Moving on to code review. 🔍 中文说明门禁复查（由维护者完成端到端验证后通过
进入代码审查。🔍 — Qwen Code · qwen3.8-max Reviewed at |
|
Code review re-run — Aug 24, still against FindingsThe scope change, the warning, the per-session reply anchor, the
None of these interleavings were exercised by the maintainer's harness (its own "not covered" list names cancel-flush and the deferred modes), which is how the E2E pass and the round-5 findings can both be true at once. Also open — not code-review findings, but they gate merge:
Files changed (9)
Testing evidence — the PR's own CI (this run never executes PR code)
All 24 check-runs on the head completed — nothing failed, nothing pending. Green CI proves the suite passes; it cannot settle the findings above either way, since R5-8/R5-9 show part of the suite pinning the wrong outcome. Behavioural evidence for the primary claims comes from the maintainer's real-environment harness (comment of Aug 12): session isolation, concurrent Sandboxed verification would settle what remains: 中文说明针对 结论：scope 修改、WARNING、按 session 的回复锚点、
维护者的测试装置没有覆盖这些交错时序（其"未覆盖"清单自己列出了取消 flush 与延迟模式），这正是 E2E 通过和 round-5 发现可以同时成立的原因。 另外两个非代码审查发现、但卡合并的事项：与 main 的冲突（#8440 的 测试证据：头部提交 24 个 check 全部完成——无失败、无进行中（表格见上）。绿 CI 只证明套件通过；鉴于 R5-8/R5-9 表明部分套件固定了错误结果，它无法裁决上述发现。主要行为证据来自维护者 8 月 12 日的真实环境对照验证（会话隔离、并发锚点、孤儿清理、变异测试）——此处为引用归属，不是本次运行复跑。作者自报的 345 单测与绿色 CI 一致，但属作者声明。无人端到端验证过的：上述延迟链交错、 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 3/5 — my read of the code hasn't moved an inch since Aug 12: the direction is right, the core fix is real and maintainer-verified, and the round-5 interleaving defects are real too. What changed is not the code reading but who owns the decision — the maintainer has decided, and this run executes that decision instead of deferring to it again. The Aug 12 deferral set a condition: if the maintainer decided the interleaving defects are acceptable to land now and fix forward, say so explicitly, and the gate would follow that call on re-run. The call is on record, and it is about as explicit as it gets: @wenshao approved this exact SHA on Aug 12 — five days after the round-5 review posted, and after publishing his own end-to-end verification against a real protocol harness — enabled auto-merge (squash) seconds later, and re-triggered So this run approves, pinned to
Two hard blockers remain before anything actually merges, and neither is something an approval can change:
@Eric-GoodBoy-Tech — the ball is on your side: merge 中文说明置信度：3/5 —— 我对代码的判断自 8 月 12 日以来没有变化：方向正确，核心修复真实有效且经维护者实环境验证，round-5 指出的交错缺陷也真实存在。变化的不是代码判断，而是决定权的归属——维护者已经做出决定，本次运行执行这个决定，而不是再次上交。 8 月 12 日的 defer 提出过条件：若维护者决定交错缺陷可以先合并、后续修复（fix forward），请明确表态，门禁将在下次运行时遵循。表态已在记录中，而且再明确不过：@wenshao 于 8 月 12 日批准了这个 SHA（在 round-5 发布五天后、在其本人用真实协议测试装置完成端到端验证之后），数秒后开启了 auto-merge（squash），今天又在批准仍然有效的状态下重新触发了 因此本次运行批准，锚定在
在任何东西真正合并之前仍有两个硬阻塞，都不是批准能改变的：
@Eric-GoodBoy-Tech —— 球在你这边：把 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
Code Review — PR #82413 Critical · 2 Suggestion Critical1. Stale per-session reply anchor survives cancelled prompt
Issue: 2. msgSeqMap entry orphaned forever
Issue: The 3. purgeSingleScopeOrphans deletes live single-scope routing entries
Issue: Purge runs unconditionally and deletes Suggestion4. Unnecessary as-unknown-as cast
Issue: 5. Test bypasses setReplyMsgId
Issue: Test writes directly to Automated review by Qwen Code |
Code Review — restore per-group session isolation under thread scopeReviewed at One under-advertised upside worth putting in the PR body: moving off 1.
|
|
感谢这个 PR！在 review 之前，想邀请你顺便把 QQ 频道的 Web Shell 管理支持也补上——目前 Web Shell 的频道管理 UI 只支持 DingTalk / WeCom / Feishu，QQ 还只能通过 需要两处改动：
这样用户就能在 Web Shell 里直接创建和管理 QQ 频道实例了。可以作为这个 PR 的一部分，也可以单独开一个 PR，你来定。 |
…hans, guard single purge - Release sessionReplyMsgId and streamState in onPromptEnd so a cancelled turn cannot hand its reply anchor to the next prompt (deferred completions are left for the in-flight flush's .then() to finish, preserving the residual-buffer re-flush) - Wrap anchor release in releaseSessionReplyAnchor() to also delete msgSeqMap entries no longer referenced by any session or the chat's current replyMsgId - Skip purgeSingleScopeOrphans when sessionScope is 'single' (the route is live state, not an orphan) - Drop the defensive as-unknown-as cast on router cleanup APIs and update router mocks in tests - Exercise the setReplyMsgId guard in stream tests instead of writing the reply map directly
ReviewRemoving the forced Two defects reproduce with tests against this branch, plus a scope gap and a lint failure. 1.
|
…ct TTL
- Set the per-session reply anchor in onPromptStart from envelope.messageId
(the same event.id setReplyMsgId stores) instead of opportunistically
capturing from the chat-level entry on the first chunk, which could
re-capture another user's msgId for slow turns past the 5-minute TTL
- Store {msgId, timestamp} in sessionReplyMsgId and drop stale anchors at
use time, so long streams no longer send chunks with an expired msg_id
- Update the qqbot.md sample config to thread, consistent with the new
default and the Session Isolation section
|
Thanks for the round-2 review — three of the five items overlap with round 1 and were already addressed in Already fixed in
Fixed in
Verification: |
…inuity - Move releaseSessionReplyAnchor after the final-segment send in onResponseComplete and after the residual re-flush settles in the deferred path, so msg_seq is not reset to 1 mid-reply (QQ dedupes on msg_id+msg_seq and silently drops the tail) - Use releaseSessionReplyAnchor for the TTL-expiry path in onResponseChunk instead of a raw delete (prevents msgSeqMap orphans) - Exclude explicit 'single' scope from the groupAllPolicy WARNING (it is global sharing, not per-user fragmentation) - Persist after dropping an orphaned msgSeqMap counter, matching the other mutation paths - Add regression tests: single-scope purge no-op, expired-anchor final segment, and keep existing anchor lifecycle tests green
|
All 7 items addressed in Critical 1 & 2 (msg_seq reset) — the anchor is now released after the final send settles, not before:
Suggestion 3 — the TTL-expiry path in Suggestion 4 — the Suggestion 5 — Suggestion 6 & 7 — regression tests added: single-scope purge is a no-op; expired-anchor final segment goes out as an active send. Verification: |
…ored-seq helper
- purgeSingleScopeOrphans: exact-match `${this.name}:__single__` instead of
suffix-matching the whole shared router, so a thread-scope QQ channel no
longer deletes sibling channels' live single-scope routes in daemon mode
- stderr template literal: drop the stray source newline that emitted \n\n
- extract isMsgIdAnchoredBySession() as the single source of truth for the
'still anchored' invariant shared by releaseSessionReplyAnchor and
setReplyMsgId
- onSessionDied test: anchor the session via onPromptStart and assert the
release drops the sessionReplyMsgId entry
…tream state - mirror isMsgIdAnchoredBySession guard at the two remaining msgSeqMap delete sites (startReplyMsgIdCleanup tick and sendMessage's expired-entry branch) so a live session anchored past the 5-minute TTL keeps its msg_seq counter and the tail send cannot reset to seq 1 (QQ dedup drops it) - give streamState entries a per-session turn generation bumped on onPromptStart; onResponseChunk drops a stale entry from a previous turn (deferred send parked in pendingStreamDelete) instead of appending the new turn's chunks to the old turn's buffer / delivering under its msgId, and a stale release can no longer delete the new turn's anchor - purge test: seed a sibling-channel ':__single__' key so the exact-match guard is actually exercised (a suffix-match mutant stays green otherwise) - qualify the anchor comments: they cover only the idle-flush path, not blockStreaming mode where onResponseChunk returns immediately - log a stderr line when the per-session anchor is dropped for TTL expiry, so a fallback to active send (and ACTIVE_MSG_DISABLED) is attributable to the TTL root cause instead of a phantom admin toggle
… drop, pin seq ordering - stale-state branch in onResponseChunk now also clears flushingSessions and pendingStreamDelete: they belong to the old turn's in-flight chain, whose identity guards fail against the fresh entry and would never delete them, stranding the session so the new turn's onResponseComplete/onPromptEnd both early-return and the buffer is never flushed (silent reply loss, probe: sendCalls 1 -> 2) - new test: drops stale state from a previous turn and clears its flush flags - new test: keeps msg_seq of an expired replyMsgId that is still session-anchored (startReplyMsgIdCleanup guard) - new test: final segment continues the msg_seq counter of its session anchor (pins release-after-send ordering: a future refactor moving release above the final send resets seq to 1 and QQ dedup drops the reply tail)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Test Plan (not a blocker): 288 tests pass — this review observed 306 passed; 288 passed — this review observed 306 passed.
中文说明
Test Plan（非阻断）：288 tests pass — this review observed 306 passed; 288 passed — this review observed 306 passed。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
… flag hygiene, real test coverage - releaseSessionReplyAnchor gains an expectedMsgId identity check: all six deferred-chain release points (flushAndTrack .then() else, .catch() permanent-failure, pending retry exhaustion, both non-pending retry exhaustion paths, onResponseComplete) now pass state.msgId captured at send start, so a chain settling after the successor prompt overwrote the anchor releases only its own turn's anchor and msgSeqMap entry — never the successor's (probe: successor anchor msg-B destroyed before, preserved after) - stale-state drop in onResponseChunk also clears flushedSessions (round-7 missed it): a cross-turn stale wasFlushed=true otherwise strangles the new turn's zero-chunk reply via the fullText dedup path, and logs dropped superseded-turn chars so tail truncation is observable - handleGroupDelRobot msgSeqMap.delete now guarded by isMsgIdAnchoredBySession (the last unguarded site) - tests: fix three vacuous flushAndTrack error-path tests (single await Promise.resolve() never drained the rejection chain — catch ran after assertions; RETRY_EXHAUSTED/ACTIVE_MSG_DISABLED asserted 'keeps streamState' against source that deletes it, now flipped + mutation-verified), rebuild the overwritten-replyMsgId test to actually reach the setReplyMsgId guard, seed the expired-entry guard tests, add lifecycle / stale-timer / two-session same-msgId / retry-exhaustion flag / re-flush-to-completion / size-cap in-flight / tool-call anchor / boundary anchor-survival tests and handleGroupDelRobot cleanup coverage (314 tests)
|
Qwen Code review timed out. Qwen review timed out after 10800 seconds (of the 180-minute budget). For large PRs, retry with a longer timeout by commenting: |
Address wenshao review QwenLM#4: - channel-settings-store: add 'chat_thread' to the accepted sessionScope set (github/gitlab already default to it; the docs and the constructor warning advertise it, but the settings API rejected it with channel_settings_invalid_config) + accept/reject tests - qqbot.md: split the Session Isolation sentence — /clear and /cancel reach follows sessionScope; permission-request answers are gated by chat+thread (and shared-target), independent of scope - QQChannel.ts: drop reviewer name from a comment (release-before-delete)
|
Thanks for review #4. Items 1, 3, 6 fixed in
Verification: |
🩺 serve daemon A/BBuilt the PR base vs this PR head ✅ No response changes against the PR base across 4 scenario(s). — Qwen Code · serve A/B |
|
Qwen Code review did not complete successfully. Qwen review aborted with an API error before posting comments. A transient error is retried automatically; if you are seeing this, retry with |
|
@qwen-code /review |
|
Hi @wenshao — gentle nudge: the round-4 fixes (items 1, 3, 6 in |
| if (state.msgId !== undefined) { | ||
| this.releaseSessionReplyAnchor(sessionId, state.msgId); | ||
| } | ||
| this.streamState.delete(sessionId); |
There was a problem hiding this comment.
[Critical] The superseded-turn branch releases the anchor while the old turn's send is still in flight. The release cascade keeps the msgSeqMap counter (live-flush guard), then the entry and flags are deleted — and when the in-flight send succeeds, flushAndTrack's .then() early-returns at current !== state without ever re-running the release. No other deletion path targets the old msgId again (chat entry and anchors moved on), so the counter is orphaned permanently in memory and in the persisted QQ state. — Failure scenario: turn 1's tail flush in flight, turn 2's first chunk takes this branch; probe-verified: msgSeqMap['msg-A']=1 survives with zero references and survives restarts (restoreQQState reloads it). One persisted orphan per supersede-with-in-flight-settle occurrence. This is one site of a family (with the settle-ordering, onSessionDied, and boundary-teardown orphans); the shared fix is a post-settle re-check — e.g. re-run releaseSessionReplyAnchor(sessionId, state.msgId) in flushAndTrack's .finally() after the flushingSessions marker clears (identity-guarded, idempotent).
中文说明
superseded 分支在旧一轮发送仍在途时释放锚点：release 级联因 in-flight 守卫保留 msgSeqMap 计数器，随后条目与标志被删除；而在途发送成功时，flushAndTrack 的 .then() 在 current !== state 处提前返回，永远不会再次执行释放。其他任何删除路径都不会再触及旧 msgId（chat 条目与锚点已指向新消息），计数器在内存与持久化 QQ state 中永久成为孤儿。失败场景：轮次 1 尾部刷新在途，轮次 2 首个分片走此分支；已用探针验证：msgSeqMap['msg-A']=1 在零引用下存活并跨重启存活。这是同一族缺陷的一个点（与 settle 顺序、onSessionDied、boundary teardown 的孤儿同源）；共同修复是 settle 后复查——例如在 flushAndTrack 的 .finally() 清掉 flushingSessions 之后重跑 releaseSessionReplyAnchor(sessionId, state.msgId)（带身份守卫、幂等）。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| if (state.msgId !== undefined) { | ||
| this.releaseSessionReplyAnchor(sessionId, state.msgId); | ||
| } | ||
| if (s === state) { |
There was a problem hiding this comment.
[Critical] The terminal-settle cleanup treats pendingStreamDelete as "the turn is over", but this PR's new onResponseBoundary defer branch (~1618) sets it MID-TURN (tool-call/plan/permission events). A boundary-deferred chain that settles with an empty buffer therefore releases the session reply anchor while the turn is still running. — Failure scenario (probe-verified): flush in flight when onResponseBoundary fires → pendingStreamDelete.add; send resolves → this else branch releases the anchor mid-turn; every later response window snaps no anchor → sendMessage falls back to the chat-level replyMsgId → the reply is re-parented onto another user's message (the exact race this PR removes), or with active messages disabled the passive send fails ACTIVE_MSG_DISABLED and the reply is dropped. Suggested fix: record why the session is parked (e.g. pendingStreamDelete as Map<string, 'terminal' | 'boundary'>) and skip the release/flushedSessions/turnCounter cleanup when the park came from onResponseBoundary.
中文说明
终态 settle 清理把 pendingStreamDelete 当作“回合已结束”，但本 PR 新增的 onResponseBoundary 延迟分支（~1618）会在回合进行中设置它（tool-call/plan/permission 事件）。因此 boundary 延迟的链在缓冲为空时 settle 会在回合仍在进行时释放会话回复锚点。失败场景（探针验证）：boundary 触发时刷新在途 → pendingStreamDelete.add；发送完成 → 此 else 分支在回合进行中释放锚点；之后每个响应窗口都无法捕获锚点 → sendMessage 回退到 chat 级 replyMsgId → 回复被重新挂到其他用户的消息上（正是本 PR 要消除的串线），或在禁用主动消息的群里被动发送以 ACTIVE_MSG_DISABLED 失败、回复被丢弃。修复建议：记录挂起原因（如 pendingStreamDelete 改为 Map<string, 'terminal' | 'boundary'>），当挂起来自 onResponseBoundary 时跳过释放/清理。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| this.streamOrphanBuffer.delete(sessionId); | ||
| const anchorEntry = this.sessionReplyMsgId.get(sessionId); |
There was a problem hiding this comment.
[Critical] The stale-entry branch deletes streamOrphanBuffer and sends only fullText — but while a stale entry persists, every chunk of the CURRENT turn is stashed in streamOrphanBuffer, and fullText contains only post-last-boundary text (the bridge clears its chunk collection at each responseBoundary). Chunks orphaned before the last boundary are silently lost. The normal-path delete (~1721) and the onResponseBoundary immediate-path delete (~1632) share the assumption. — Failure scenario (probe-verified): turn 1's tail flush fails transiently and its entry survives retry backoff; turn 2's chunks stash; a tool-call boundary clears the bridge's collected chunks; turn 2 completes while the old entry stands → this branch deletes turn 2's entire stashed text and sends only post-boundary fullText; observed 'new-turn-text-1' never sent. Suggested fix: while the stale entry persists the orphan buffer is an in-order superset of fullText — deliver orphaned || fullText under the current turn's anchor, or defer completion until the stale chain settles.
中文说明
stale 分支删除 streamOrphanBuffer 且只发送 fullText——但只要旧条目仍存，当前轮的所有分片都暂存在 streamOrphanBuffer 中，而 fullText 只包含最后一个 boundary 之后的文本（bridge 在每个 responseBoundary 清空分片集合）。最后一个 boundary 之前被孤立的分片会被静默丢失。normal 路径的删除（~1721）与 onResponseBoundary 立即路径的删除（~1632）同样基于该假设。失败场景（探针验证）：轮次 1 尾部刷新瞬时失败、条目在重试回退中存活；轮次 2 分片被暂存；tool-call boundary 清空 bridge 已收集分片；轮次 2 在旧条目仍存时完成 → 本分支删除轮次 2 全部暂存文本、只发送 boundary 后的 fullText；观测到 'new-turn-text-1' 从未发出。修复建议：旧条目存活期间 orphan buffer 是 fullText 的有序超集——按当前轮锚点发送 orphaned || fullText，或将完成推迟到旧链 settle。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| if (!this.isMsgIdAnchoredBySession(oldEntry.msgId)) { | ||
| this.msgSeqMap.delete(oldEntry.msgId); | ||
| } |
There was a problem hiding this comment.
[Critical] Permanent msgSeqMap orphan via the retention path. The overwrite keeps the counter (this anchor guard); onPromptStart's identity-free release (~1093) keeps it (live-flush guard); the settle-time release runs BEFORE .finally() clears flushingSessions and keeps it again — and no post-settle re-check exists anywhere. — Failure scenario (probe-verified on the exact concurrent-overwrite race this PR targets): user A's tail flush in flight at turn end; user B's setReplyMsgId keeps the counter; turn 2's onPromptStart releases the anchor — cascade keeps the counter; the chain settles, the pending-else release keeps it a second time; then entry + marker are gone. msgSeqMap['msg-A'] survives with zero references, serialized on every save and re-loaded across restarts. On origin/main the overwrite-time delete was unconditional, so this class did not exist. Family fix with R5-1: re-run the cascade for the chain's own msgId in .finally() after the marker clears.
中文说明
保留路径导致的永久 msgSeqMap 孤儿：覆盖写入时该锚点守卫保留计数器；onPromptStart 的无身份释放（~1093）因 in-flight 守卫保留；settle 时的释放先于 .finally() 清 flushingSessions 执行、再次保留——且任何地方都没有 settle 后复查。失败场景（就在本 PR 要解决的同群并发覆盖竞态上探针验证）：A 的尾部刷新在回合结束时在途；B 的 setReplyMsgId 保留计数器；轮次 2 onPromptStart 释放锚点——级联保留；链 settle 时 pending-else 释放第二次保留；随后条目与标志消失。msgSeqMap['msg-A'] 零引用存活、每次保存被序列化、跨重启恢复。origin/main 上覆盖时删除是无条件的，因此此前不存在此类问题。与 R5-1 同族修复：在 .finally() 清标志后对链自身 msgId 重跑级联。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| // would re-resolve msg_seq from 1 (QQ dedupes on msg_id + msg_seq). | ||
| this.releaseSessionReplyAnchor(sessionId); | ||
| this.streamState.delete(sessionId); |
There was a problem hiding this comment.
[Critical] onSessionDied's release-before-delete ordering (required by the cbd7faa review round to stop the msg_seq reset under live sends) retains the msgSeqMap counter while a flush is in flight — but the teardown immediately afterwards removes every path that could ever re-release it, and the settling chain's identity guards never re-run the release → permanent persisted counter orphan. — Failure scenario (probe-verified; the flip fix passes the suite): session dies mid-turn while its flush is in flight and the chat entry moved on; the guard keeps the counter; the deletes remove anchor/entry/marker; success settle early-returns at current !== state (no release), transient failure skips on the identity guard; only a permanent failure cleans up. Every remaining msgSeqMap.delete site keys off references that moved on — the counter is serialized and re-loaded across restarts. Suggested fix (closes the shared hole behind R5-1/R5-7/R5-31 too): in flushAndTrack, when the identity guard fails (.then() early-return and the .catch() skip), still call releaseSessionReplyAnchor(sessionId, state.msgId) — idempotent, and expectedMsgId keeps a successor's anchor untouched. Note the extended onSessionDied test (~1824 in stream.test.ts) pins the orphan as intended and must be flipped together with the fix.
中文说明
onSessionDied 的先释放后删除顺序（cbd7faa 评审轮为避免在途发送下 msg_seq 重置而要求的）会在刷新在途时保留 msgSeqMap 计数器——但紧随其后的清理移除了所有可能再次释放它的路径，而 settle 链的身份守卫永远不会重跑释放 → 永久持久化计数器孤儿。失败场景（探针验证；修复翻转后全套测试通过）：会话在刷新在途且 chat 条目已切换时死亡；守卫保留计数器；删除移除锚点/条目/标志；成功 settle 在 current !== state 处提前返回（不释放），瞬时失败被身份守卫跳过；只有永久失败才清理。其余 msgSeqMap.delete 点都依赖已移走的引用——计数器被序列化并跨重启恢复。修复建议（同时闭合 R5-1/R5-7/R5-31 的共因洞）：在 flushAndTrack 中身份守卫失败时（.then() 提前返回与 .catch() 跳过处）仍调用 releaseSessionReplyAnchor(sessionId, state.msgId)——幂等，且 expectedMsgId 不动后继锚点。注意 stream.test.ts ~1824 的 onSessionDied 测试把孤儿当作预期行为固定了下来，需随修复一并翻转。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| // Exhaustion releases the pending flag and the flushed record... | ||
| expect(pendingStreamDelete.has('sess-1')).toBe(false); | ||
| expect(flushedSessions.has('sess-1')).toBe(false); |
There was a problem hiding this comment.
[Suggestion] The pending-retry-exhaustion path's deleteTurnCounterIfOwned cleanup (QQChannel.ts:~1502) is pinned by no test: this test drives exactly that path and asserts the pending flag, flushed record, anchor release and seq cascade — but never turnCounter. Mutant re-verified: deleting the call leaves 345/345 green. — Failure scenario: a refactor dropping or relocating the call leaves a stale per-session turnCounter entry after an abandoned deferred turn, and the successor-ownership semantics at this site are unverified. The sibling onToolCall exhaustion test (~931) has the same omission. Suggested fix: after the exhaustion loop assert expect((chp['turnCounter'] as Map<string, number>).has('sess-1')).toBe(false); (and in the onToolCall sibling).
中文说明
pending 重试耗尽路径的 deleteTurnCounterIfOwned 清理（QQChannel.ts:~1502）没有任何测试固定：本测试恰好驱动该路径并断言了 pending 标志、flushed 记录、锚点释放与 seq 级联——唯独从不检查 turnCounter。突变体复核：删除该调用后 345/345 仍全绿。失败场景：丢弃或搬移该调用的重构会在被放弃的延迟轮之后留下陈旧的会话 turnCounter 条目，且该调用点的后继所有权语义未经验证。兄弟的 onToolCall 耗尽测试（~931）同样遗漏。修复建议：耗尽循环后断言 expect((chp['turnCounter'] as Map<string, number>).has('sess-1')).toBe(false);（onToolCall 兄弟测试同样处理）。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| // so the next window's fresh streamState entry reuses the same msgId. | ||
| expect(sessionAnchors.has('sess-1')).toBe(true); |
There was a problem hiding this comment.
[Suggestion] The boundary test pins anchor RETENTION but never anchor REUSE — the post-boundary delivery's msg_id/msg_seq are asserted nowhere, although the stated purpose of the retention is "reuses the same msgId". Current code sends 'final' under msg-A (the stateless complete path reads the fresh session anchor); no test anywhere pins a post-boundary send's routing. — Failure scenario: if a future edit drops the sessionReplyMsgId read on the stateless complete path, moves the release ahead of the send, or re-parents onto the chat-level entry, the post-boundary final segment ships as an active send — dropped outright in groups with active messages disabled (ACTIVE_MSG_DISABLED) or re-parented onto a concurrent message — and the test stays green. Suggested fix: after onResponseComplete assert expect(body['msg_id']).toBe('msg-A') and expect(body['msg_seq']).toBe(1) next to the content assertion.
中文说明
boundary 测试固定了锚点的保留，却从未固定锚点的复用——boundary 之后投递的 msg_id/msg_seq 在任何地方都没有断言，而保留机制的既定目的正是“复用同一 msgId”。当前代码在 msg-A 下发送 'final'（无状态完成路径读取新的会话锚点）；没有任何测试固定 boundary 后发送的路由。失败场景：若未来改动删掉无状态完成路径对 sessionReplyMsgId 的读取、把释放提前到发送之前、或回退到 chat 级条目，boundary 后的最后分段会作为主动发送发出——在禁用主动消息的群中被直接丢弃（ACTIVE_MSG_DISABLED）或被挂到并发消息上——而测试依然绿灯。修复建议：在 onResponseComplete 之后、content 断言旁补 expect(body['msg_id']).toBe('msg-A') 与 expect(body['msg_seq']).toBe(1)。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| // Sibling channel's live user-scope route: the owned-by-name guard | ||
| // (entry.target?.channelName === this.name) must NOT purge it — a |
There was a problem hiding this comment.
[Suggestion] Both new purge tests (here and ~532) document an "owned-by-name guard" (entry.target?.channelName === this.name) that does not exist in purgeSingleScopeOrphans — the method never reads entry.target (its getAll cast doesn't even include it); sibling-key protection comes solely from an exact key match against the channel's own ":single" key (the 3-part purge was rescoped out in f5c3317, leaving these comments stale). — Failure scenario: the assertions are correct, but the comments are the only documentation of the sibling-safety invariant, and the method's doc comment anticipates a future extension to age out 3-part keys — a maintainer writing that extension would trust an ownership check that isn't there; a prefix/suffix-matching implementation would silently reset sibling channels' sessions with no guard to catch it. Suggested fix: rewrite the comments to name the real mechanism — the sibling key survives because the purge only matches keys exactly equal to the channel's own single-scope key; 3-part keys are never candidates.
中文说明
两个新增 purge 测试（此处与 ~532）都描述了一个“按名称所有权守卫”（entry.target?.channelName === this.name），但 purgeSingleScopeOrphans 中并不存在——该方法从不读取 entry.target（其 getAll 转型甚至不含该字段）；兄弟键保护完全来自与本渠道自身 ":single" 键的精确匹配（三段键 purge 已在 f5c3317 中收缩移除，这些注释因此过期）。失败场景：断言本身正确，但这些注释是兄弟安全不变量的唯一文档，且方法 doc 注释预期未来会扩展为淘汰三段键——写该扩展的维护者会信任一个不存在的所有权检查；前缀/后缀匹配的实现会静默重置兄弟渠道的会话而无任何守卫能捕获。修复建议：把注释改写为真实机制——兄弟键之所以保留，是因为 purge 只匹配与本渠道自身 single-scope 键完全相等的键；三段键从来不是候选。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| expect(flushedSessions.has('session-pc')).toBe(false); | ||
| expect(turnCounter.has('session-pc')).toBe(false); |
There was a problem hiding this comment.
[Suggestion] The permanent-catch teardown tests pin deleteTurnCounterIfOwned only with MATCHING turns (this test seeds turn 1 / counter 1; the parallel stream.test.ts deferred-permanent test seeds 5/5), so the ownership guard at QQChannel.ts:~1456 is unpinned — mutant re-verified: replacing it with an unconditional turnCounter.delete() keeps 345/345 green. — Failure scenario: in production the guard is load-bearing: a deferred turn whose tail flush fails permanently after a successor started (the stash branch keeps the old entry alive while onPromptStart bumps the counter) would have the successor's counter deleted by the dead turn's settle — stale-state detection resets to 0, the successor's live entry is misread as superseded, and its buffered chunks are dropped. Probe-verified: under the mutant the successor's counter is erased and no PR test observes it. Suggested fix: add a variant where the successor bumped the counter (turnCounter 2, state.turn 1) and assert the counter is still 2 after the permanent settle.
中文说明
permanent-catch 清理测试只用匹配的轮次固定 deleteTurnCounterIfOwned（本测试播种 turn 1/计数器 1；平行的 stream.test.ts 延迟永久失败测试播种 5/5），因此 QQChannel.ts:~1456 的所有权守卫未被固定——突变体复核：替换为无条件 turnCounter.delete() 后 345/345 仍全绿。失败场景：生产中该守卫是承重的：后继轮已开始后尾刷新永久失败的延迟轮（stash 分支保活旧条目、onPromptStart 抬升计数器），会被死轮的 settle 删掉后继计数器——陈旧检测归零，后继轮的存活条目被误判为 superseded，其缓冲分片被丢弃。探针验证：突变体下后继计数器被抹除且无 PR 测试能观测到。修复建议：增加后继轮已抬升计数器的变体（turnCounter 2、state.turn 1），断言永久 settle 后计数器仍为 2。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| expect(st.turn).toBe(2); | ||
| // The old turn's flags are cleared so turn 2's onResponseComplete / | ||
| // onPromptEnd are not short-circuited into a silent reply loss. |
There was a problem hiding this comment.
[Suggestion] The stale-drop test pins flag clearing and fresh-entry fields but never pins that the drop must PRESERVE the successor's turnCounter — mutant probed: adding this.turnCounter.delete(sessionId); to onResponseChunk's stale branch survives 345/345. — Failure scenario: a cleanup refactor deleting the counter the successor's onPromptStart just bumped leaves turn 2 streaming under entries stamped turn:2 with an empty turnCounter: at completion onResponseComplete computes currentTurn=0, sees state.turn (2) !== 0, misclassifies its own live entry as stale and re-sends the entire fullText (duplicate delivery) while the parked residual still flushes. Probe flips with the suggested assertion. Suggested fix: assert expect((chp['turnCounter'] as Map<string, number>).get('sess-1')).toBe(2); after the stale-drop assertions.
中文说明
stale-drop 测试固定了标志清除与新条目字段，却从未固定“删除必须保留后继轮 turnCounter”——突变体探测：向 onResponseChunk 的 stale 分支加入 this.turnCounter.delete(sessionId); 后 345/345 仍全绿。失败场景：某个清理重构若删掉后继轮 onPromptStart 刚抬升的计数器，轮次 2 会以 turn:2 条目继续流式而 turnCounter 为空：完成时 onResponseComplete 算出 currentTurn=0，发现 state.turn (2) !== 0，把自己的存活条目误判为陈旧并重发整个 fullText（重复投递），而挂起的残余仍在刷新。所建议的断言可使探针翻转。修复建议：在 stale-drop 断言后补 expect((chp['turnCounter'] as Map<string, number>).get('sess-1')).toBe(2);。
— qwen3.8-max via Qwen Code /review (v0.21.7)
|
@qwen-code resolve |
|
@qwen-code /resolve |
|
Qwen Code resolved the merge conflicts, but could not push to Root causemain's PR #8440 (
Textual or semanticSemantic, but orthogonal: both sides edited the same adjacent lines while modifying different enums, so the resolution is the union: groupPolicy: new Set(['disabled', 'allowlist', 'pairing', 'open']),
sessionScope: new Set(['user', 'thread', 'chat_thread', 'single']),The doc table is the union of row contents, re-padded to the widest cell (main's What is load-bearing
What I could not verifyNo build, lint, or tests were run. Two notes for CI:
中文说明根因：main 上的 PR #8440（群组配对功能，提交 3edecac）与本 PR 的提交 f5c3317 相撞：前者在 性质：语义冲突但改动正交——双方修改同一相邻行里的不同枚举，解决方案为两者的并集（见上代码块）。文档表取行内容并集并按最宽单元格重新对齐，逐格比对确认合并结果与两侧的差异仅在对方修改的行。 关键点：两个枚举值必须同时保留，缺一会破坏对应侧的校验或测试；合并后 channel-base 的 未能验证：本次未运行构建/测试。注意：同文件中本 PR 的枚举改动与 #8517 对 |
Maintainer verification — real end-to-end run, not just unit testsI built a real environment for this PR locally and ran Harness — how "real" this is
1. Session isolation — confirmed, and
|
main (8fd0162) |
this PR (f5c3317) | |
|---|---|---|
| persisted routing keys | qq:U1:GA, qq:U2:GB, qq:U3:U3, qq:U2:GA |
qq:GA, qq:GB |
| group A, other member asks | RECALL=NONE ❌ |
RECALL=PP-ALPHA ✅ |
| group B / DM ask | RECALL=NONE ✅ |
RECALL=NONE ✅ |
DM /clear |
"This clears the shared session for everyone who shares it. Re-send with confirm…" ❌ |
"Session cleared." ✅ |
The important finding: on main the forced sessionScope: 'single' never reaches the router. start.ts:361 (and daemon-worker.ts:516) call router.setChannelScope(name, config.sessionScope) with the parsed config — before QQChannel's constructor rewrites its own copy. So main prints WARNING: … Forcing sessionScope to 'single' to ensure shared group context. while routing stays user-scoped. Two consequences, both reproduced above:
groupAllPolicy: "all"never actually delivered shared group context — group traffic was fragmented per sender, which is the exact opposite of what the override existed to guarantee.QQChannelstill believes it issingle, so everysessionScope === 'single'gate inside it fires against user-scoped routing — a DM gets treated as a shared session and/cleardemands an extraconfirm.
So one line of the PR description overstates a symptom: with channel start / daemon mode, main does not leak context across groups and into DMs — that only happens if the user set sessionScope: "single" explicitly (which the docs did recommend for groupAllPolicy). The real main defect is fragmentation + the split brain. The fix is still correct and strictly better; the description just credits it for fixing the wrong half. Worth a one-line edit before merge.
2. Concurrent streaming reply anchor — reproduced exactly
Identical config on both sides (sessionScope: "user", no groupAllPolicy, so no scope forcing is involved). U1 (msg-A) starts a slow streamed answer in group A; 3s later U2 sends msg-B in the same group, overwriting the chat-level replyMsgId. The table is the real request bodies received at /v2/groups/GA/messages:
main: U1's segments 2 and 3 go out undermsg-B— the bot visibly answers the wrong person — and both users' replies share onemsg_id's seq run (1→5).- this PR:
msg-A→ seq 1/2/3,msg-B→ seq 1/2/3. No cross-threading.
3. Orphan purge, mutation test, and the declared breaking change
- Upgrade path: seeded a genuine "single-scope era" deployment by running
mainwithsessionScope: "single"(→ realqq:__single__insessions.json), then restarted the sameQWEN_HOMEwithsessionScoperemoved. PR:[QQ:qq] Purged 1 orphaned single-scope session mapping(s),Ready (0 sessions), routing switches cleanly toqq:GA, and the first post-restart message answers normally.main: the deadqq:__single__entry survives the restart and a user-scope key is added alongside it. - Mutation (necessity check): deleting only
defaultSessionScope: 'thread'frompackages/channels/qqbot/src/index.tson the PR branch and rebuildingdistregresses zero-config deployments straight back to user scope (qq:U1:GA…, group ARECALL=NONE). That line is load-bearing, not cosmetic. It also confirms the new WARNING text matches real behavior. - Breaking change reproduced as declared: a plain
@mentiondeployment with nogroupAllPolicyand nosessionScopealso moves fromqq:U1:GA(per-member private) toqq:GA(whole-group shared). U2 goes fromRECALL=NONEonmaintoRECALL=PP-ALPHAon the PR. Intended, documented in the PR body — but it needs to reach release notes, not just the PR description.
4. Static checks (head)
| Check | Result |
|---|---|
npx vitest run packages/channels/qqbot |
7 files, 345 passed |
npx vitest run packages/cli/src/serve/channel-settings-store.test.ts |
42 passed |
tsc --noEmit -p packages/channels/qqbot/tsconfig.json |
clean |
eslint on changed sources |
clean |
prettier --check on changed sources + docs |
clean |
Not covered
blockStreaming: "on"(the documented anchor blind spot) — not exercised end-to-end.- The cancel-flush behavior change — only covered by the PR's unit regression test, not driven through a live turn here.
- Daemon mode (
qwen serve --channel) — I droveqwen channel start.daemon-worker.ts:516has the identicalsetChannelScopepattern, so the same reasoning applies, but I did not run it. - Windows / Linux; real QQ credentials.
Recommendation
LGTM for merge. The routing fix, the per-session reply anchor, and the orphan purge all behave as claimed against a real runtime, and the mutation test shows the plugin default is required rather than incidental. Two asks before merging: (a) correct the "context leaked across groups and into DMs" framing in the PR body — that is the sessionScope: "single" case, not what main does by default; (b) make sure the user → thread default flip lands in the release notes, since it hits every QQ deployment that omits sessionScope, not only groupAllPolicy users.
中文版
维护者验证 —— 本地真实环境端到端跑通，不只是单测
我在本地搭了真实环境，把 main 和 PR 分支并排跑了一遍。结论：修复符合声明，而且 main 上的实际缺陷比 PR 描述写的还要糟。
验证环境有多"真"
- 被测对象：两个 worktree 各自编译出的
dist/起真实的qwen channel start qq进程（已核对：head 的dist/QQChannel.js含purgeSingleScopeOrphans/sessionReplyMsgId，base 的没有）。base =8fd0162，head =f5c3317。 - QQ 侧：本地伪造的 QQ 开放平台，说的是真协议 ——
POST /app/getAppAccessToken、GET /gateway、真实 WSS 网关（HELLO / IDENTIFY / READY / HEARTBEAT / DISPATCH）、POST /v2/groups/:id/messages与/v2/users/:id/messages。每一次外发请求体都记进 JSONL 账本，msg_id/msg_seq的证据就来自这里。 - 产品代码零改动。唯一的钩子是一个
--require预加载，把发往*.qq.com的 TLS 连接在传输层重定向到127.0.0.1。仍然是真 HTTPS、真 SNI、真主机名校验（证书带qq.comSAN），所以validateGatewayUrl()的wss://+*.qq.com硬边界是被真正执行的，不是被绕过。 - 模型：录制型 OpenAI 兼容服务。机器人回答
RECALL=<它拿到的历史里出现过的口令>，所以"它答出什么"直接等于"它被喂了哪一份会话历史"。 - macOS 26.6 / Node v24.18.1。
1. 会话隔离 —— 确认修复，且 main 存在"精神分裂"
配置：groupAllPolicy: "all"、senderPolicy/groupPolicy: open、不写 sessionScope。流量：U1 在群 A 告诉机器人口令 → U2 在群 B 问 → U3 私聊问 → U2 在群 A 问 → U3 私聊发 /clear。
main (8fd0162) |
本 PR (f5c3317) | |
|---|---|---|
| 持久化路由键 | qq:U1:GA、qq:U2:GB、qq:U3:U3、qq:U2:GA |
qq:GA、qq:GB |
| 群 A 内另一位成员来问 | RECALL=NONE ❌ |
RECALL=PP-ALPHA ✅ |
| 群 B / 私聊来问 | RECALL=NONE ✅ |
RECALL=NONE ✅ |
私聊 /clear |
"This clears the shared session for everyone who shares it…" ❌ | "Session cleared." ✅ |
关键发现：main 上那个强制 sessionScope: 'single' 根本没传到 router。start.ts:361（以及 daemon-worker.ts:516）用的是解析后、构造器之前的 config 去调 router.setChannelScope(name, config.sessionScope)，而构造器只改了 QQChannel 自己那份副本。于是 main 一边打印 WARNING: … Forcing sessionScope to 'single'，一边照旧按 user 作用域路由。两个后果上面都复现了：
groupAllPolicy: "all"从来没有真正提供过"群内共享上下文" —— 群消息按发送人被切碎了，恰恰是这个强制想保证的反面。- QQChannel 自己仍认为是
single，所以它内部所有sessionScope === 'single'的判断都在对着 user 作用域的路由生效 —— 私聊被当成共享会话，/clear要求二次confirm。
因此 PR 描述里有一句话夸大了症状：在 channel start / daemon 模式下，main 并不会跨群、跨私聊泄漏上下文 —— 那只发生在用户显式写了 sessionScope: "single" 的部署上（而当时文档确实推荐这么配）。main 真正的缺陷是"碎片化 + 精神分裂"。修复本身依然正确且严格更优，只是描述把功劳记在了错误的那一半上，合并前值得改一行。
2. 并发流式回复锚点 —— 精确复现
两侧配置完全相同（sessionScope: "user"，无 groupAllPolicy，不牵涉任何强制逻辑）。U1（msg-A）在群 A 发起慢速流式回答；3 秒后 U2 在同一个群发 msg-B，覆盖了 chat 级 replyMsgId。表格是 /v2/groups/GA/messages 收到的真实请求体：
main：U1 回复的第 2、3 段挂到了msg-B上 —— 群里看到的就是"机器人回错了人" —— 而且两个人的回复共用了同一条msg_id的 seq 序列（1→5）。- 本 PR：
msg-A→ seq 1/2/3，msg-B→ seq 1/2/3，互不串线。
3. 孤儿清理、变异测试、以及 PR 自己声明的破坏性变更
- 升级路径：先用
main+sessionScope: "single"真跑出一份"single 时代"的部署状态（sessions.json里是真实的qq:__single__），再把sessionScope删掉、用同一个QWEN_HOME重启。PR：[QQ:qq] Purged 1 orphaned single-scope session mapping(s)、Ready (0 sessions)，路由干净切到qq:GA，重启后第一条消息正常回复。main：那条死掉的qq:__single__熬过重启活了下来，旁边还新建了一个 user 作用域的键。 - 变异测试（必要性验证）：在 PR 分支上只删掉
packages/channels/qqbot/src/index.ts里的defaultSessionScope: 'thread'并重新编译dist，零配置部署立刻退回 user 作用域（qq:U1:GA…，群 ARECALL=NONE）。这一行是承重墙，不是装饰。同时也验证了新 WARNING 的措辞与真实行为一致。 - 破坏性变更如实复现：一个没有
groupAllPolicy、也没写sessionScope的普通@mention部署，同样会从qq:U1:GA（每人私有）变成qq:GA（整群共享）。U2 在main下得到RECALL=NONE，在 PR 下得到RECALL=PP-ALPHA。这是有意为之、PR 正文也写了 —— 但它需要进 release notes，而不只是躺在 PR 描述里。
4. 静态检查（head）
| 检查项 | 结果 |
|---|---|
npx vitest run packages/channels/qqbot |
7 个文件，345 通过 |
npx vitest run packages/cli/src/serve/channel-settings-store.test.ts |
42 通过 |
tsc --noEmit -p packages/channels/qqbot/tsconfig.json |
干净 |
改动源文件 eslint |
干净 |
改动源文件 + 文档 prettier --check |
干净 |
未覆盖
blockStreaming: "on"（文档化的锚点盲区）—— 没有端到端验证。- 取消时 flush 部分回复的行为变更 —— 只有 PR 自带的单测覆盖，我没有在真实回合里跑。
- Daemon 模式（
qwen serve --channel）—— 我跑的是qwen channel start。daemon-worker.ts:516是完全相同的setChannelScope写法，推理同样成立，但我没有实跑。 - Windows / Linux；真实 QQ 凭据。
结论
同意合并。 路由修复、按 session 的回复锚点、孤儿清理，在真实运行时下都与声明一致，变异测试也证明了插件默认值是必需项而非顺手加的。合并前两个请求：(a) 修正 PR 正文里"跨群、跨私聊泄漏上下文"的说法 —— 那是 sessionScope: "single" 的情况，不是 main 的默认行为；(b) 确保 user → thread 的默认值翻转写进 release notes，因为它影响的是每一个省略 sessionScope 的 QQ 部署，而不只是用 groupAllPolicy 的用户。
|
@qwen-code /triage |
|
Sandboxed verification: Skipped because the PR has merge conflicts, so refs/pull/8241/merge is unavailable — resolve conflicts and re-run. 中文 — 判定：
|
|
Triage re-run completed — approved this round. The maintainer's landing decision (approval on 门禁本轮批准了：维护者的落地决定（批准该 SHA + auto-merge）正是上次 defer 请求的明确表态，门禁遵循该决定、锚定该提交批准；round-5 发现作为 fix-forward 工作保留在记录中。但距离真正合并还差一步：作者需把 The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
|
@qwen-code /triage |
|
Sandboxed verification: Skipped because the PR has merge conflicts, so refs/pull/8241/merge is unavailable — resolve conflicts and re-run. 中文 — 判定：
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Approving per the maintainer's explicit land-and-fix-forward decision on this SHA (approval + auto-merge) — the gate promised to follow that call on re-run. The round-5 items remain on record as fix-forward work; the branch still cannot merge until the author merges main (conflict). Details in the Stage 3 comment. ✅
|
@qwen-code /resolve |
|
Qwen Code attempted to resolve merge conflicts but the run did not complete successfully. Check the workflow run for full logs. |
|
Thanks for this! The PR is approved and CI is green, but it can't be merged right now because it has merge conflicts with The conflicts are small — only two files, one hunk each:
One semantic note while you sync: Please rebase onto the latest |
|
我在精确 head
验证结果：外置竞态 probe 建议把 park reason 区分成 主问题是真实的，移除 constructor scope override、QQ 默认 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Deferred under the convergence posture (round 6, not a blocker) — recorded, not requested in this round:
packages/channels/qqbot/src/QQChannel.ts:1663 — [probe] Stale-branch no-anchor fallback is untested (mutant survives)packages/channels/qqbot/src/QQChannel.ts:247 — [probe] blockStreaming note misstates the mechanism (captured-but-unused, not never-captured)
Convergence: round 6 posted 9 inline comment(s), 1 of them reported for the first time. Findings keep coming back to the same files: packages/channels/qqbot/src/QQChannel.ts (findings in round 5; 1 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
收敛姿态下延后（第 6 轮，非阻断）——已记录，本轮不要求修改：共 2 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 6 轮发布了 9 条行内评论，其中 1 条是首次提出。发现反复回到同一批文件：packages/channels/qqbot/src/QQChannel.ts（第 5 轮已出过发现，本轮又有 1 条）。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.0)
| // Release before the delete: the release guard scans streamState + | ||
| // flushingSessions for a live flush, so it must still see this entry | ||
| // (and marker) or it would drop the msg_seq counter under a send still | ||
| // in flight — QQ dedupes on msg_id + msg_seq and silently drops the | ||
| // tail. The expectedMsgId identity check keeps a successor turn's | ||
| // anchor untouched while still cascading this turn's counter away. | ||
| if (state.msgId !== undefined) { | ||
| this.releaseSessionReplyAnchor(sessionId, state.msgId); | ||
| } |
There was a problem hiding this comment.
[Critical] R5-1: Round-6 re-check at the unchanged head f5c33173 — still stands. The superseded-turn branch releases the anchor while the old turn's send is still in flight. The release cascade keeps the msgSeqMap counter (live-flush guard), then the entry and flags are deleted — and when the in-flight send succeeds, flushAndTrack's .then() early-returns at current !== state without ever re-running the release. No other deletion path targets the old msgId again, so the counter is orphaned permanently in memory and in the persisted QQ state — one persisted orphan per supersede-with-in-flight-settle occurrence. This round's chunk agents re-derived the identical mechanism independently at this commit.
Suggested fix (family with R5-4/R5-5/R5-7): re-run releaseSessionReplyAnchor(sessionId, state.msgId) for the chain's own msgId in flushAndTrack's .finally(), after the flushingSessions marker clears — identity-guarded, idempotent.
中文说明
第 6 轮复查（HEAD f5c33173 未变化）——仍然存在。superseded 分支在旧一轮发送仍在途时释放锚点：release 级联因 in-flight 守卫保留 msgSeqMap 计数器，随后条目与标志被删除；在途发送成功时，flushAndTrack 的 .then() 在 current !== state 处提前返回，永远不会再次执行释放。其他任何删除路径都不会再触及旧 msgId，计数器在内存与持久化 QQ state 中永久成为孤儿——每次"supersede 且在途发送成功"产生一个持久化孤儿。本轮分块 agent 已在本 commit 独立重新推导出相同机制。修复建议（与 R5-4/R5-5/R5-7 同族）：在 flushAndTrack 的 .finally() 清掉 flushingSessions 之后，对链自身的 msgId 重跑 releaseSessionReplyAnchor(sessionId, state.msgId)——带身份守卫、幂等。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| } else { | ||
| // No own anchor (proactive turn / expired TTL) — nothing to | ||
| // release, and an unconditional release here would delete a | ||
| // successor turn's anchor. See .catch() release points. | ||
| if (state.msgId !== undefined) { | ||
| this.releaseSessionReplyAnchor(sessionId, state.msgId); | ||
| } |
There was a problem hiding this comment.
[Critical] R5-2: Round-6 re-check at the unchanged head f5c33173 — still stands. The terminal-settle cleanup treats pendingStreamDelete as "the turn is over", but this PR's new onResponseBoundary defer branch (~1618) sets it MID-TURN (tool-call/plan/permission events). A boundary-deferred chain that settles with an empty buffer therefore releases the session reply anchor while the turn is still running — re-verified at this commit by this round's agents. Every later response window then snaps no anchor, so sendMessage falls back to the chat-level replyMsgId and the reply is re-parented onto another user's message (the exact race this PR removes); with active messages disabled the passive send fails ACTIVE_MSG_DISABLED and the reply is dropped. Round 5 probe-verified this at the same commit.
Suggested fix: record why the session is parked (e.g. pendingStreamDelete as Map<string, 'terminal' | 'boundary'>) and skip the release/flushedSessions/turnCounter cleanup when the park came from onResponseBoundary.
中文说明
第 6 轮复查（HEAD 未变化）——仍然存在。终态 settle 清理把 pendingStreamDelete 当作"回合已结束"，但本 PR 新增的 onResponseBoundary 延迟分支（~1618）会在回合进行中设置它（tool-call/plan/permission 事件）。boundary 延迟的链在缓冲为空时 settle，会在回合仍在进行时释放会话回复锚点——本轮 agent 已在本 commit 重新验证。之后每个响应窗口都无法捕获锚点，sendMessage 回退到 chat 级 replyMsgId，回复被重新挂到其他用户的消息上（正是本 PR 要消除的串线）；若群禁用主动消息，被动发送以 ACTIVE_MSG_DISABLED 失败、回复被丢弃。第 5 轮已在同一 commit 用探针验证。修复建议：记录挂起原因（如把 pendingStreamDelete 改为 Map<string, 'terminal' | 'boundary'>），当挂起来自 onResponseBoundary 时跳过释放/清理。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| // This turn's own anchor (set by onPromptStart) may still be valid — | ||
| // use it instead of falling back to the racy chat-level entry, which | ||
| // this PR exists to avoid re-parenting onto. | ||
| this.streamOrphanBuffer.delete(sessionId); | ||
| const anchorEntry = this.sessionReplyMsgId.get(sessionId); |
There was a problem hiding this comment.
[Critical] R5-3: Round-6 re-check at the unchanged head f5c33173 — still stands. The stale-entry branch deletes streamOrphanBuffer and sends only fullText — but while a stale entry persists, every chunk of the CURRENT turn is stashed in streamOrphanBuffer, and fullText contains only post-last-boundary text (the bridge clears its chunk collection at each responseBoundary — re-confirmed in AcpBridge this round). Chunks orphaned before the last boundary are silently lost. Round-5 probe at this commit: turn 1's tail flush fails transiently and its entry survives retry backoff; turn 2's chunks stash; a tool-call boundary clears the bridge's collected chunks; turn 2 completes while the old entry stands → this branch deletes turn 2's entire stashed text and sends only post-boundary fullText; observed 'new-turn-text-1' never sent.
Suggested fix: while the stale entry persists the orphan buffer is an in-order superset of fullText — deliver orphaned || fullText under the current turn's anchor, or defer completion until the stale chain settles.
中文说明
第 6 轮复查（HEAD 未变化）——仍然存在。stale 分支删除 streamOrphanBuffer 且只发送 fullText——但只要旧条目仍存，当前轮的所有分片都暂存在 streamOrphanBuffer 中，而 fullText 只包含最后一个 boundary 之后的文本（bridge 在每个 responseBoundary 清空分片集合——本轮已在 AcpBridge 中重新确认）。最后一个 boundary 之前被孤立的分片会被静默丢失。第 5 轮在同一 commit 的探针：轮次 1 尾部刷新瞬时失败、条目在重试回退中存活；轮次 2 分片被暂存；tool-call boundary 清空 bridge 已收集分片；轮次 2 在旧条目仍存时完成 → 本分支删除轮次 2 全部暂存文本、只发送 boundary 后的 fullText；观测到 'new-turn-text-1' 从未发出。修复建议：旧条目存活期间 orphan buffer 是 fullText 的有序超集——按当前轮锚点发送 orphaned || fullText，或将完成推迟到旧链 settle。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| // Release before the deletes so the release guard still sees this | ||
| // session's entry (with its buffered/flushing state) and can keep the | ||
| // msg_seq counter while a live flush owns it — a delete-then-release | ||
| // order would drop the counter under an in-flight send and its tail | ||
| // would re-resolve msg_seq from 1 (QQ dedupes on msg_id + msg_seq). | ||
| this.releaseSessionReplyAnchor(sessionId); |
There was a problem hiding this comment.
[Critical] R5-5: Round-6 re-check at the unchanged head f5c33173 — still stands. onSessionDied's release-before-delete ordering (required by the cbd7faa review round to stop the msg_seq reset under live sends) retains the msgSeqMap counter while a flush is in flight — but the teardown immediately afterwards removes every path that could ever re-release it, and the settling chain's identity guards never re-run the release → permanent persisted counter orphan, serialized on every save and re-loaded across restarts. Round-5 probe verified the orphan and its flip fix (suite green); this round's layer walk re-mapped the residual to this entry at the same commit.
Suggested fix (closes the shared hole behind R5-1/R5-7 too): in flushAndTrack, when the identity guard fails (.then() early-return and the .catch() skip), still call releaseSessionReplyAnchor(sessionId, state.msgId) — idempotent, and expectedMsgId keeps a successor's anchor untouched. Note the extended onSessionDied test plus the R5-8/R5-9 pins must flip together with the fix.
中文说明
第 6 轮复查（HEAD 未变化）——仍然存在。onSessionDied 的先释放后删除顺序（cbd7faa 评审轮为避免在途发送下 msg_seq 重置而要求的）会在刷新在途时保留 msgSeqMap 计数器——但紧随其后的清理移除了所有可能再次释放它的路径，而 settle 链的身份守卫永远不会重跑释放 → 永久持久化计数器孤儿，每次保存被序列化、跨重启恢复。第 5 轮探针已验证该孤儿及其翻转修复（套件绿灯）；本轮分层走查再次把该残留映射到本条目。修复建议（同时闭合 R5-1/R5-7 的共因洞）：在 flushAndTrack 中身份守卫失败时（.then() 提前返回与 .catch() 跳过处）仍调用 releaseSessionReplyAnchor(sessionId, state.msgId)——幂等，且 expectedMsgId 不动后继锚点。注意扩展的 onSessionDied 测试与 R5-8/R5-9 固定点需随修复一并翻转。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| this.streamState.delete(sessionId); | ||
| this.flushedSessions.delete(sessionId); | ||
| if (remaining) { |
There was a problem hiding this comment.
[Critical] R5-6: Round-6 re-check at the unchanged head f5c33173 — still stands. onResponseComplete's normal path takes over a parked deferred flush (clears the re-parked timer, delivers the residual) but never clears pendingStreamDelete; onPromptEnd's flag-based early return then lets the stale flag survive into the next turn, whose first successful flush's .then() consumes it mid-turn (s === state) and runs the terminal-settle cleanup. Round-5 probe (one-line fix flips): turn 2's reply delivered TWICE (msg-B seq 1 and seq 2 — QQ dedup does not suppress it, the chat-entry guard kept the counter); if more chunks follow instead, they compute currentTurn=0 vs state.turn=2 → the superseded branch drops the buffered middle. This round's R6-1 verification re-confirmed the unconditional pending-flag consumption mechanics at this commit.
| this.streamState.delete(sessionId); | |
| this.flushedSessions.delete(sessionId); | |
| if (remaining) { | |
| this.streamState.delete(sessionId); | |
| this.flushedSessions.delete(sessionId); | |
| this.pendingStreamDelete.delete(sessionId); | |
| if (remaining) { |
Add the missing pendingStreamDelete.delete(sessionId) next to the streamState/flushedSessions deletes.
中文说明
第 6 轮复查（HEAD 未变化）——仍然存在。onResponseComplete 的 normal 路径接管被挂起的延迟刷新（清掉重新挂起的定时器、投递残余缓冲）但从不清除 pendingStreamDelete；onPromptEnd 基于该标志提前返回，使陈旧标志存活到下一轮——下一轮首次成功刷新的 .then() 会在回合进行中消费它（s === state）并执行终态清理。第 5 轮探针（一行修复即翻转）：轮次 2 的回复被投递两次（msg-B seq 1 与 seq 2——QQ 去重不会抑制，chat 条目守卫保留了计数器）；若后续还有分片，则 currentTurn=0 对比 state.turn=2 → superseded 分支丢弃中段缓冲。本轮 R6-1 验证已在本 commit 重新确认"无条件消费 pending 标志"的机制。修复建议：在 streamState/flushedSessions 删除旁补上缺失的 this.pendingStreamDelete.delete(sessionId);（见上方 suggestion）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| // on the replaced entry, dropping the segment's text. | ||
| this.pendingStreamDelete.add(sessionId); | ||
| return; | ||
| } | ||
| this.streamState.delete(sessionId); | ||
| this.flushingSessions.delete(sessionId); | ||
| this.pendingStreamDelete.delete(sessionId); | ||
| this.flushedSessions.delete(sessionId); | ||
| this.streamOrphanBuffer.delete(sessionId); |
There was a problem hiding this comment.
[Critical] R5-7: Round-6 re-check at the unchanged head f5c33173 — still stands. onResponseBoundary's non-flushing teardown destroys a previous turn's parked deferred-flush entry (buffer + retry/re-flush timer + pendingStreamDelete) without ever releasing that entry's reply anchor — the anchor's msgSeqMap counter, retained by the cascade guard precisely because the parked entry still held it, becomes a permanent persisted orphan. Distinct trigger from the settle-race and session-death orphans: here the parked chain's timer is cleared, so no settle ever runs again and a settle-time re-release fix cannot catch it. Round-5 probe verified at this commit (the fix flips and passes 346/346): chain settles and re-arms the park (residual + 2s timer + flag); a new message arrives in the park window; the new turn's first responseBoundary fires before the park timer → this teardown clears everything; no reference to msg-A remains anywhere and no future release can target it (the TTL net scans only anchor maps).
Suggested fix: capture the stale entry's msgId (guarded on state.turn !== currentTurn) and release after the deletes.
中文说明
第 6 轮复查（HEAD 未变化）——仍然存在。onResponseBoundary 的非刷新清理会摧毁上一轮被挂起的延迟刷新条目（缓冲+重试/再刷新定时器+pendingStreamDelete），却从不释放该条目的回复锚点——锚点对应的 msgSeqMap 计数器正因为挂起条目仍持有它而被级联守卫保留，遂成为永久持久化孤儿。与 settle 竞态、会话死亡孤儿的触发点不同：此处挂起链的定时器被清除，settle 永远不会再发生，settle 时补释放的修复无法覆盖。第 5 轮已在本 commit 用探针验证（修复翻转后 346/346 通过）：链 settle 后重新挂起（残余+2s 定时器+标志）；挂起窗口内新消息到达；新轮首个 responseBoundary 先于挂起定时器触发 → 本清理删除一切；msg-A 在任何地方都不再有引用，也没有未来释放能触及它（TTL 安全网只扫锚点映射）。修复建议：捕获陈旧条目的 msgId（以 state.turn !== currentTurn 为守卫），在删除之后释放。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| // The dead turn's entry still held a buffered residual when the release | ||
| // ran, so the guard kept the counter (it cannot reset to 1). | ||
| expect(seqMap.get('msg-1')).toBe(2); |
There was a problem hiding this comment.
[Critical] R5-8: Round-6 re-check at the unchanged head f5c33173 — still stands. This assertion pins the orphaned-counter outcome that the onSessionDied release-before-delete ordering produces (R5-5): nothing is in flight in this test (synchronous; timers never advanced; mockSendQQMessage never invoked; flushingSessions empty), yet the guard keeps the counter solely because the entry's buffer is truthy — and after onSessionDied returns nothing references msg-1 anywhere, while serializeQQState persists it: a permanent persisted orphan. Two costs: (1) the suite pins the bug as intended semantics with a comment that will mislead the next maintainer; (2) any fix of the R5-5 orphan makes this assertion fail — the fix cannot land green until the test is flipped. File unchanged at this commit (suite 345/345 green; walked this round).
Suggested fix: together with the R5-5 production fix, flip to expect(seqMap.has('msg-1')).toBe(false) and rewrite the comment. (Do not apply this test flip alone — it is red against today's code.)
中文说明
第 6 轮复查（HEAD 未变化）——仍然存在。该断言固定了 onSessionDied 先释放后删除顺序（R5-5）所产生的孤儿计数器结果：本测试中没有任何在途发送（同步执行、从未推进定时器、mockSendQQMessage 从未被调用、flushingSessions 为空），守卫仅因条目 buffer 非空而保留计数器——onSessionDied 返回后 msg-1 在任何地方都无引用，而 serializeQQState 仍会持久化它：永久持久化孤儿。两个代价：(1) 测试套件把该 bug 当作预期语义固定下来，注释会误导后续维护者；(2) 任何对 R5-5 孤儿的修复都会让该断言失败——不翻转测试修复无法绿灯合入。文件在本 commit 未变化（套件 345/345 绿灯；本轮已走查）。修复建议：随 R5-5 生产修复一并改为 expect(seqMap.has('msg-1')).toBe(false) 并重写注释。（请勿单独应用该测试翻转——对当前代码它是红的。）
— qwen3.8-max via Qwen Code /review (v0.22.0)
| // The tail send settles; the chain's settle-time release runs under the | ||
| // same in-flight marker, so the counter survives to the end. | ||
| resolveTailSend!(mockResponse(true)); | ||
| await drain(); | ||
|
|
||
| expect(seqMap.get('msg-A')).toBe(2); |
There was a problem hiding this comment.
[Critical] R5-9: Round-6 re-check at the unchanged head f5c33173 — still stands. This test pins the orphaned-counter outcome of the settle-time retention (R5-4) as intended behavior: after the tail send settles, msgSeqMap['msg-A'] === 2 is asserted although nothing references msg-A anymore (session anchor → msg-B, chat entry → msg-B, no streamState entry, no flushing marker — round-5 probe verified 0 references; every cleanup path keys off references that moved on). When the settle-orphan fix lands (post-settle re-check release), seqMap.has('msg-A') becomes false and this assertion fails — the suite turns red against the fix; the comment ("the counter survives to the end") documents the orphan as spec, inviting the fix author to preserve the leak. releaseSessionReplyAnchor's own JSDoc names the anti-guarantee ("otherwise they accumulate forever in memory and in the persisted QQ state"). File unchanged at this commit (79/79 green this round).
Suggested fix: with the R5-4 fix, flip the post-settle block to expect(seqMap.has('msg-A')).toBe(false) and drop the "survives to the end" narrative; keep the mid-flight assertions (seqMap.get('msg-A') === 2 before resolveTailSend), which correctly pin the dedup-drop protection. (Do not apply this test flip alone — it is red against today's code.)
中文说明
第 6 轮复查（HEAD 未变化）——仍然存在。该测试把 settle 时保留（R5-4）所产生的孤儿计数器当作预期行为固定下来：尾部发送 settle 后断言 msgSeqMap['msg-A'] === 2，但此时 msg-A 已无任何引用（会话锚点 → msg-B、chat 条目 → msg-B、无 streamState 条目、无 flushing 标志——第 5 轮探针验证 0 引用；所有清理路径都依赖已移走的引用）。settle 孤儿修复（settle 后补释放）落地后，seqMap.has('msg-A') 变为 false，该断言失败——套件对修复变红；注释（"计数器存活到最后"）把孤儿写成规范，诱导修复者保留泄漏。releaseSessionReplyAnchor 的 JSDoc 明确指出反面（"否则它们会在内存和持久化 QQ state 中永远累积"）。文件在本 commit 未变化（本轮 79/79 绿灯）。修复建议：随 R5-4 修复把 settle 后断言改为 expect(seqMap.has('msg-A')).toBe(false) 并删除"存活到最后"的说法；保留在途断言（resolveTailSend 前 seqMap.get('msg-A') === 2），它们正确地固定了去重丢包保护。（请勿单独应用该测试翻转——对当前代码它是红的。）
— qwen3.8-max via Qwen Code /review (v0.22.0)
| if (this.pendingStreamDelete.has(sessionId)) { | ||
| this.pendingStreamDelete.delete(sessionId); | ||
| this.flushedSessions.delete(sessionId); | ||
| this.deleteTurnCounterIfOwned(state, sessionId); | ||
| } | ||
| return; |
There was a problem hiding this comment.
[Critical] R6-1: The flushAndTrack .catch() settle blocks consume pendingStreamDelete (and flushedSessions/turn counter) without any identity/turn check, so a superseded turn's failing flush chain steals a successor turn's parked-completion flag — the successor's residual tail is then silently lost. Failure scenario (probe-verified at this commit): turn 1 completes while its flush F1 is in flight → onResponseComplete parks (pendingStreamDelete.add); turn 2 runs and completes while its own flush F2 is in flight → parked again; F1 then settles with a failure — permanent (RETRY_EXHAUSTED/ACTIVE_MSG_DISABLED/FALLBACK_FAILED) or transient (RATE_LIMITED): the pending cleanup runs unconditionally and deletes turn 2's park flag and flushedSessions record. F2's .then() then sees pendingStreamDelete.has === false, skips the residual re-flush, and the entry is left with a non-empty buffer, no timer, no flag — turn 2's tail is discarded by the next turn's stale-drop ("dropping N chars of superseded turn") or stranded until disconnect. Realistic under QQ rate limits / slow token refresh, and it compounds with R5-10 for the concurrent-flush leg.
Witness (scratch-tree probe at f5c33173):
PERMANENT (RETRY_EXHAUSTED): pendingStreamDelete.has('s1') after F1 reject: false ← successor park flag stolen
'turn2-tail' never sent; stderr: dropping 10 chars of superseded turn 2 for s1
TRANSIENT (RATE_LIMITED): same theft; 'turn2-tail' never sent
FLIP (identity-guarded cleanup): both probes pass; qqbot suites 267/267 green
| if (this.pendingStreamDelete.has(sessionId)) { | |
| this.pendingStreamDelete.delete(sessionId); | |
| this.flushedSessions.delete(sessionId); | |
| this.deleteTurnCounterIfOwned(state, sessionId); | |
| } | |
| return; | |
| if ( | |
| current === state && | |
| this.pendingStreamDelete.has(sessionId) | |
| ) { | |
| this.pendingStreamDelete.delete(sessionId); | |
| this.flushedSessions.delete(sessionId); | |
| this.deleteTurnCounterIfOwned(state, sessionId); | |
| } | |
| return; |
The suggestion guards the permanent branch; apply the same identity guard to the transient branch's pendingStreamDelete.delete(sessionId) below (return before consuming the flag when current !== state), or scope the park flag to the turn that set it.
中文说明
flushAndTrack 的 .catch() settle 块在消费 pendingStreamDelete（以及 flushedSessions/turn 计数器）时没有任何身份/轮次检查，因此被取代轮次的失败刷新链会偷走后继轮次的挂起完成标志——后继轮次的残余尾部随后被静默丢失。失败场景（已在本 commit 用探针验证）：轮次 1 完成时其刷新 F1 仍在途 → onResponseComplete 挂起（pendingStreamDelete.add）；轮次 2 运行并在其自身刷新 F2 在途时完成 → 再次挂起；F1 随后以失败 settle——永久（RETRY_EXHAUSTED/ACTIVE_MSG_DISABLED/FALLBACK_FAILED）或瞬时（RATE_LIMITED）：pending 清理无条件执行，删除轮次 2 的挂起标志与 flushedSessions 记录；F2 的 .then() 随后看到 pendingStreamDelete.has === false，跳过残余再刷新，条目带着非空缓冲、无定时器、无标志被留下——轮次 2 的尾部被下一轮的 stale-drop 丢弃（"dropping N chars of superseded turn"）或一直滞留到断连。在 QQ 限流/慢 token 刷新下现实可达，并与 R5-10 叠加构成并发刷新路径。证据（f5c33173 探针树）：永久与瞬时两分支均观测到挂起标志被偷、'turn2-tail' 从未发出；身份守卫修复后两个探针均通过、qqbot 套件 267/267 绿灯。修复建议：上方 suggestion 为永久分支补身份守卫；请对下方瞬时分支的 pendingStreamDelete.delete(sessionId) 应用同样守卫（current !== state 时在消费标志前返回），或把挂起标志改为按轮次作用域。
— qwen3.8-max via Qwen Code /review (v0.22.0)
5 participants
What this PR does
Restores per-group session isolation for the QQ Bot channel. Removes the forced
sessionScope: 'single'override that the constructor applied whenevergroupAllPolicywaskeywordorall(introduced during PR #6457 review); the channel now respects the configured scope and warns when a non-shared scope is combined with full-message group mode. Reply streaming is anchored per session so concurrent users in the same group no longer corrupt each other'smsg_idthreading. Orphaned persisted routing entries from the old single-scope layout are purged on startup, and the QQ channel now declaresthreadas its default session scope so zero-config deployments get per-group shared context by default.Why it's needed
With
groupAllPolicy: "all", every group, every user, and every DM was collapsed into one shared__single__session — context leaked across groups and into DMs, and the bot answered one group's questions using another group's history. The forced scope also produced follow-on bugs (shared-session destruction, brokenrouter.getTarget) that were patched around instead of fixed. ThereplyMsgIdconcurrency issue that motivated the override was never actually resolved by it, only hidden.Reviewer Test Plan
How to verify
npx vitest run packages/channels/qqbot— 7 files, 345 tests pass.groupAllPolicy: "all"and no explicitsessionScope, the channel must not emit the scope-forcing WARNING and must route group messages per group (:) and DMs per user (:).msg_id(no cross-threading) — covered by the new stream tests./cancelor steer) must flush the buffered partial reply before tearing down stream state — partial output is not dropped (regression test pins this).Evidence (Before & After)
N/A — internal routing/session behavior, no user-visible UI change. Before: all groups/DMs shared one
__single__session. After: per-group shared sessions with cross-group isolation.Tested on
Environment (optional)
Unit tests only:
npx vitest run packages/channels/qqbot— 345 passed;tsc --noEmit -p packages/channels/qqbot/tsconfig.json— clean;eslinton changed source — clean;prettier— clean.Risk & Scope
sessionScope: the plugin'sdefaultSessionScopeis nowthreadinstead of the globaluserdefault (config-utils.tsfillsrawConfig.sessionScope || plugin.defaultSessionScope || 'user'). This applies to every QQ config that omitssessionScope— not justgroupAllPolicy: keyword/allusers — including plaingroupPolicy: "allowlist"+requireMentiondeployments. Such users go fromuser(per-sender contexts) tothread(per-group shared context)./clear,/cancel, permission-request answers, and the!host-shell gate apply to it for the whole group. WithsenderPolicy: "open"and noallowedUsers, any group member can clear/cancel the shared session and approve tool permissions for it. If members should not share history or control each other's turns, setsessionScope: "user"or restrict membership (senderPolicy/allowedUsers) — see the updated Session Isolation doc section.__single__orphans. On startup the channel removes persisted<channel>:__single__routing entries left over from the forced single-scope era (PR feat(qqbot): group message handling and cron-msg-experimental #6457); their history is not migrated (it cannot be split per group). Three-part user-scope keys are not purged — they age out naturally, so a zero-config deployment upgrading fromusertothreadscope keeps its sessions instead of losing them.onPromptEndnow flushes the buffered partial reply before tearing down stream state (previously the idle timer flushed it asynchronously; the round-1 anchor-release change had dropped it). Cancelled turns keep their partial output. This applies to/cancel, steer, and/clearalike — a cleared turn also flushes any buffered partial reply (documented, intended behavior).blockStreaming: "on"mode never captures a per-session anchor (documented limitation).Linked Issues
Fixes #8238
中文说明
修复 QQ Bot channel 的会话隔离：删除 groupAllPolicy=keyword/all 时构造器强制 sessionScope='single' 的逻辑（PR #6457 review 期间引入），改为尊重配置并对非共享 scope 组合发出 WARNING。流式回复按 session 锚定 msg_id，同群并发用户不再互相串线；启动时清理旧的 single 孤儿持久化条目；QQ channel 声明默认 sessionScope 为 thread，零配置部署默认获得每群共享上下文。
背景：强制 single 把所有群、所有用户、私聊折叠进一个 single 会话，导致跨群/私聊上下文互相泄漏；且当年为掩盖 replyMsgId 并发问题引入的强制并未真正解决该问题。
破坏性变更：任何省略 sessionScope 的 QQ 部署（不只是 groupAllPolicy=keyword/all）默认 scope 从 user 变为 thread——群聊线程是共享会话，/clear、/cancel、权限审批、! 门控对全群生效；senderPolicy=open 且无 allowedUsers 时任何群成员可清共享会话/批准工具权限，需要时请设置 sessionScope=user 或限制 allowedUsers。旧 single 持久化会话启动时清理（历史无法按群拆分迁移）。取消 turn 时会先 flush 缓冲的部分回复再清理状态（部分输出不丢失）。
验证：单元测试 345 通过，tsc clean，eslint/prettier clean，均在 macOS 本地完成；Windows/Linux 未测。真实 QQ 凭据的全链路验证不在 CI 覆盖范围。blockStreaming=on 模式不捕获 per-session 锚点（文档化限制）。