Skip to content

feat(channels)!: remove message prefix filtering - #11571

Merged
qqqys merged 1 commit into
QwenLM:mainfrom
qqqys:feat/remove-channel-message-prefix
Sep 10, 2026
Merged

feat(channels)!: remove message prefix filtering#11571
qqqys merged 1 commit into
QwenLM:mainfrom
qqqys:feat/remove-channel-message-prefix

Conversation

@qqqys

@qqqys qqqys commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

Removes configurable message-prefix filtering from channels. Eligible messages now follow the normal sender, group, mention and pairing policies without requiring or stripping an extra prefix. Command help and permission instructions use direct slash commands, and channel settings no longer offer the removed option.

Why it's needed

Prefix matching rewrote message bodies in the shared runtime and in several channel-specific paths, with extra handling for command menus, media, notifications and history. Removing the feature eliminates those filtering and rewriting paths. With an old messagePrefix: "/review" setting, ordinary hello is now delivered, and /review /new remains the received text instead of being rewritten into a local reset command.

Reviewer Test Plan

How to verify

  • Open channel settings and confirm there is no Message Prefix field. An unchanged legacy key can remain when other settings are saved, but the managed interface must reject adding or changing that unsupported key.
  • With a legacy prefix key still present, send ordinary text and literal /review text. Confirm normal dispatch without prefix filtering or clipping, subject to each platform's existing text normalization.
  • Use direct /help, /new, and permission replies. Confirm that help displays direct commands, session reset works normally, and permission ownership and shared-session confirmation remain enforced.
  • Verify unauthorized senders and disallowed groups are still rejected, mention-required groups still require a trigger, and ordinary first contact follows pairing policy.
  • Send captioned and captionless media, and verify both the caption and attachment survive. Synthetic media placeholders must stay out of quoted group history.
  • Exercise native todo/assignment notifications, comment aggregation and eligible group history. Their existing authorization, deduplication and event handling should continue without prefix-specific branches.

Evidence (Before & After)

  • A standalone 34-case script exercises only the shared channel runtime, with paired legacy-key and no-key configurations. Against upstream 779cfe source, 22 passed and 12 expected failures reproduced prefix filtering/clipping; against the new built artifact, all 34 passed. No DingTalk adapter is loaded by this script.
  • Full build, bundle, type checking, changed-file ESLint, Prettier and diff checks passed. The standalone branch passed 658 DingTalk tests and 188 CLI tests. The shared base and eight other channel trees are byte-identical to their already-tested versions (2,824 passing tests). The bundled CLI version and channel help also ran successfully in an isolated home.
  • This standalone change excludes the DingTalk mention-body parser fix in fix(dingtalk): preserve mention text in message bodies #11566. That follow-up will be updated and merged after this PR. No live DingTalk UI acceptance result is claimed for this removal.

Tested on

OS Status
🍏 macOS
🪟 Windows ⚠️ not tested locally
🐧 Linux ⚠️ not tested locally

Environment (optional)

Node.js 22 and an isolated worktree. Current runtime verification exercises the shared channel runtime with a local test bridge, without a live provider or external messages. The global CLI is an older version without channel commands, so it cannot exercise this runtime path.

Risk & Scope

  • Breaking change: channel-level messagePrefix filtering is removed. Existing stored keys no longer affect runtime behavior. Deployments that previously used the prefix to narrow traffic will now receive messages allowed by their remaining channel policies.
  • The generic settings compatibility rule permits an unchanged legacy key to remain when saving other settings; no configuration migration or live service change is performed by this PR.
  • Independent sender/group/mention policies, pairing, permission ownership, media provenance and deduplication are retained. The change does not add new slash commands or alter platform-specific mention parsing.
  • Not validated / out of scope: a fresh live channel round, group newline folding and image/text interleaving. Local callback verification and hosted CI are separate evidence levels.

Linked Issues

No linked issue. This PR must merge before #11566; the DingTalk mention change is deliberately excluded.

中文说明

本次改动

移除 channel 的可配置消息前缀过滤功能。符合条件的消息直接按发送者、群聊、mention 和配对策略处理,不再要求额外前缀,也不裁剪该前缀。命令帮助与权限提示直接展示斜杠命令,渠道设置不再提供已删除的选项。

原因

原有前缀匹配会在公共运行时及多个渠道内部改写正文,并为命令菜单、媒体、通知和群历史增加特殊处理。删除这个功能后,这些过滤和改写路径一并移除。即使旧配置还保留 messagePrefix: "/review",普通 hello 也会正常传递,/review /new 会保留为收到的正文,不会被裁剪成本地重置命令。

Reviewer 验证方式

  • 打开渠道设置,确认没有 Message Prefix 字段。保存其他设置时,已有旧字段可以原样保留,但管理接口必须拒绝新增或修改这个不再支持的字段。
  • 保留旧前缀配置,发送普通文字及字面的 /review 文本,确认经过平台原有文字正常化后正常传递,不被前缀过滤或裁剪。
  • 使用直接的 /help/new 和权限回复,确认帮助展示裸命令,会话重置正常,权限归属和共享会话确认仍有效。
  • 确认未授权发送者和不允许的群聊仍被拒绝,要求 mention 的群聊仍需触发,普通首次联系遵循配对策略。
  • 发送带说明和不带说明的媒体,确认说明文字和附件完整,自动生成的媒体占位符不会混入引用群历史。
  • 验证原生待办/指派通知、评论聚合和符合条件的群历史,确认原有授权、去重及事件处理正常,已移除前缀专属分支。

修复前后证据

  • 独立的 34 项脚本仅验证公共渠道运行时,成对覆盖保留旧字段和未配置字段。上游 779cfe 源码运行时为 22 项通过、12 项因前缀过滤/裁剪而预期失败;新构建产物全部 34 项通过。该脚本不加载钉钉适配器。
  • 完整构建、打包、类型检查及变更文件的 ESLint、Prettier、diff 检查通过。独立分支的 658 项钉钉测试和 188 项 CLI 测试通过;公共渠道包与另外 8 个渠道的代码与此前已验证版本逐字节一致(合计 2,824 项测试通过)。打包后的 CLI 版本查询和 channel 帮助也在隔离 home 中正常运行。
  • 本独立 PR 不包含 fix(dingtalk): preserve mention text in message bodies #11566 的钉钉 mention 正文修复。后者会在本 PR 合入后更新并合入。本次功能删除没有声称完成真实钉钉界面验收。

测试平台与环境

macOS 已测试,Windows 和 Linux 未在本地测试。使用 Node.js 22、隔离 worktree、公共渠道运行时及本地测试 bridge;当前验证没有连接真实渠道或发送外部消息。全局 CLI 是没有 channel 命令的旧版本,无法覆盖该运行时路径。

风险与范围

  • 不兼容变化:删除渠道级 messagePrefix 过滤。已有旧字段不再影响运行时行为;原先用前缀缩小消息范围的部署,升级后将接收剩余渠道策略允许的消息。
  • 通用配置兼容规则允许已有旧字段随其他设置原样保存;本 PR 不迁移配置,也不修改运行中的服务。
  • 保留独立的发送者/群聊/mention 策略、配对、权限归属、媒体来源标记和去重。本次不增加新的斜杠命令,也不修改平台 mention 解析。
  • 未验证或范围之外:新一轮真实渠道验收、群聊换行折叠及图文交错顺序。本地回调验证与远端 CI 分别报告。
  • 没有关联 issue。本 PR 先于 fix(dingtalk): preserve mention text in message bodies #11566 合入,钉钉 mention 修复明确排除在外。

BREAKING CHANGE: messagePrefix no longer filters or rewrites channel messages. Existing sender, group, mention and pairing policies still apply.
@qqqys

qqqys commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator Author

Standalone message-prefix removal verification

Tested removal commit: 60eeddc2cafd301b0ff11fa110dccffee1266d66, based on upstream ac1edef974cdfd212d964fba4e6758ca97678ec2.

A 34-case local script drives the actual shared channel runtime with paired configurations: a retained legacy messagePrefix: /review field and no prefix field. The script does not import DingTalk or depend on #11566.

  • Exact upstream 779cfe source baseline: 22 passed, 12 expected failures due to prefix filtering or clipping; all 17 unconfigured controls passed.
  • New built runtime: the identical script passed all 34 cases.
  • Covered behaviors: ordinary group/private text, literal /review /new preservation, direct /new and /help, independent sender/group/mention/DM gates, pairing, media preparation, synthetic-history exclusion, and permission ownership.
  • The old permission cases fail because filtered initial text never creates an owning session; these failures are not evidence of an authorization vulnerability.

Standalone package checks: 658 DingTalk tests and 188 CLI configuration/catalog tests passed. The shared base and eight other channel source/test trees are byte-for-byte identical to the previously validated trees, whose 2,824 tests passed (base 1,302; DWS 377; QQ 311; Feishu 288; GitHub 209; WeCom 142; Weixin 97; GitLab 61; Telegram 37).

Full build, bundle and type checking passed. The later upstream update only changed the separate review workflow/CLI implementation; CLI build and type checking plus the bundle were repeated after that update and passed. Changed-file ESLint, Prettier and diff checks passed. Bundled version and channel-help smoke checks used a temporary home/runtime directory.

The globally installed CLI is 0.10.0 and has no channel subcommand, so this is a local runtime test-script fallback. No live channel was connected, no external message was sent and no existing service was restarted. Hosted CI/review results are separate from these local results. The DingTalk mention parser is unchanged from upstream; #11566 remains a separate follow-up that must merge after this removal.

@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

Qwen Triage finishedview run. See the stage comments in this thread for the result.

Qwen Triage 已完成 —— 查看运行。结果见本线程中的各阶段评论。

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR — this one is unusually disciplined mechanically, and the thing I can't settle is not the code.

Template looks good ✓ (all required sections, bilingual, reviewer test plan filled in rather than stubbed).

Problem: this is a feature removal, not a bug fix, and there's no linked issue — so I went and found the history myself. The feature landed five days ago in #10817 (+3109/−144 across 41 files), closing #10816, a P3 feature request you also filed. The cost being removed here is observed and quantifiable rather than theoretical: it needed three Envelope fields (displayTextOffset, messagePrefixText, bypassMessagePrefix), a pair of WeakSets for once-only checking, prefixedCommand() threaded through roughly thirty user-facing strings, and per-adapter mention-run parsers (Feishu's stripLeadingMentionNames, QQ's displayTextOffset with its attacker-controlled-nick comment). The /review /new → local reset command rewrite you describe is a genuine correctness hazard, not a hypothetical. So the problem is real. What's missing is a record of the decision to remove it.

Direction: this is where I need a human, and it's the reason I'm not approving. messagePrefix shipped in stable v0.23.2 — I checked the tag directly: packages/channels/base/src/message-prefix.ts is present at v0.23.2 (3299 bytes, identical to main), and so is the docs table row. That release went out 2026-09-09, the day before this PR. So this removes a documented, user-facing setting that is in users' hands right now, and it drops two exported functions (startsWithMessagePrefix, stripMessagePrefix) from the @qwen-code/channel-base public surface. Both are public-contract changes, which the gate escalates rather than judges. The behavioural consequence for anyone who configured it is silent: their channel starts dispatching every otherwise-authorized message, and the pairing-reply noise the docs paragraph you deleted says the prefix exists to suppress comes back in busy groups. The PR body is honest about this — I'm flagging it because "honest about it" and "safe to ship without a deprecation window" are different claims. Also worth noting: AGENTS.md asks for a design doc on non-trivial work or design decisions, and a 32-file breaking removal of a shipped feature is both; there isn't one in the diff.

Size: cross-package (ten channel packages plus packages/cli), so it counts as core under the cross-package clause. Production logic 626 lines across 15 files; tests 1352 across 16; docs 5 across 1; generated/schema 0. Title is feat(channels)!: so there's no size hard block, and it's under the 1000+ advisory. You're a repo maintainer (CODEOWNERS for /packages/core/, core-area owner in issue-owners.json, write access), so AGENTS.md's maintainer exemption applies to the two-tier core gate — I'm reporting the numbers for transparency rather than as an escalation.

Approach: the scope is right and I could not find any fat in it. I checked completeness mechanically instead of by eyeball: the set of files on main referencing any prefix symbol is exactly 32, and this PR changes exactly those 32 — no leftovers, no drive-by refactors, no formatting churn. Every adapter path I traced is behaviour-identical when no prefix is configured (QQ's rawCommandText/displayText/text, Telegram's four buildEnvelope call sites after the allowRegisteredCommandBypass positional is dropped, DWS's createImEnvelope falling back to its own text = message.content.trim() default, GitHub's comment aggregation). No dead symbols either — stripBotMention, rawMessageId, commandText, alreadyPrefixed and syntheticText all still have live readers. One question worth sitting with before this merges: if the real cost is the body rewriting rather than the filter, then keeping messagePrefix as a pure drop-non-matching gate and deleting only displayTextOffset / messagePrefixText / stripLeadingMentionNames would kill the /review /new collision and most of the per-adapter complexity without breaking a setting that shipped yesterday. That's a smaller and non-breaking version of the same cleanup — genuinely worth considering, not a blocker.

Risk: no elevated risk signals — the Stage 1e revert-history path patterns matched nothing in this diff.

Continuing to the code review so whoever picks this up has the full picture in one place, but the direction question above is the one that decides it. 🙏

中文说明

感谢贡献!这个 PR 在机械执行层面异常干净,而我无法定论的部分不在代码本身。

模板完整 ✓(必填章节齐全、中英双语、Reviewer Test Plan 是认真填写的而非占位)。

问题: 这是一次功能移除而不是 bug 修复,且没有关联 issue——所以我自己去查了历史。该功能五天前才通过 #10817 落地(41 个文件,+3109/−144),关闭的是 #10816,一个同样由你提出的 P3 功能请求。这里要移除的成本是已观测、可量化的,不是理论性的:它需要三个 Envelope 字段(displayTextOffsetmessagePrefixTextbypassMessagePrefix)、一对用于「只检查一次」的 WeakSet、穿插在约三十处面向用户文案里的 prefixedCommand(),以及各渠道自己的 mention 前导解析(飞书的 stripLeadingMentionNames、QQ 带有「昵称可被攻击者控制」注释的 displayTextOffset)。你描述的 /review /new 被改写成本地重置命令,是真实的正确性隐患而非假设。所以问题是真实存在的。缺的是这个移除决定本身的记录。

方向: 这是我需要人来定夺的地方,也是我不批准的原因。messagePrefix 已经随 stable v0.23.2 发布——我直接查了 tag:packages/channels/base/src/message-prefix.tsv0.23.2 上存在(3299 字节,与 main 完全一致),文档表格行也在。那个版本发布于 2026-09-09,就在本 PR 前一天。所以这移除的是一个已在用户手中、有文档的用户可见配置项,同时还从 @qwen-code/channel-base 的公开面上删掉了两个导出函数(startsWithMessagePrefixstripMessagePrefix)。两者都属于公共契约变更,gate 对此只升级、不代为判断。对已配置该选项的用户,行为变化是静默的:他们的渠道会开始投递所有其他条件已授权的消息,而你删掉的那段文档明确说明前缀存在的目的正是抑制的配对回复噪音,会在活跃群聊里重新出现。PR 描述对此是坦诚的——我之所以仍要标出,是因为「坦诚说明」和「可以在没有废弃过渡期的情况下直接发布」是两个不同的主张。另外值得一提:AGENTS.md 要求非平凡改动或涉及设计决策时写设计文档,而一个 32 文件、移除已发布功能的破坏性变更两者都占;diff 里没有设计文档。

规模: 跨包改动(十个渠道包加 packages/cli),因此按跨包条款计入 core。生产逻辑 626 行 / 15 个文件;测试 1352 行 / 16 个文件;文档 5 行 / 1 个文件;生成/schema 0 行。标题是 feat(channels)!:,因此没有规模硬阻断,也未达到 1000+ 的大 PR 建议阈值。你是仓库维护者(/packages/core/ 的 CODEOWNERS、issue-owners.json 中 core 区域的 owner、具备 write 权限),因此 AGENTS.md 的维护者豁免适用于两层 core gate——列出这些数字是为了透明,而不是作为升级处理。

方案: 范围是对的,我没能找出任何多余部分。我用机械方式而非肉眼核对了完整性:main 上引用任何前缀相关符号的文件恰好是 32 个,而本 PR 改动的正好是这 32 个——没有残留,没有顺手重构,没有格式化噪音。我追踪过的每一条渠道路径在「未配置前缀」时行为完全一致(QQ 的 rawCommandText/displayText/text;Telegram 在去掉 allowRegisteredCommandBypass 这个位置参数后的四处 buildEnvelope 调用;DWS 的 createImEnvelope 回落到自身的 text = message.content.trim() 默认值;GitHub 的评论聚合)。也没有留下死符号——stripBotMentionrawMessageIdcommandTextalreadyPrefixedsyntheticText 都仍有活跃的读取方。有一个问题值得在合并前想一想:如果真正的成本是改写正文而不是过滤本身,那么把 messagePrefix 保留为纯粹的「不匹配即丢弃」闸门,只删掉 displayTextOffset / messagePrefixText / stripLeadingMentionNames,就能消除 /review /new 冲突和大部分按渠道特化的复杂度,同时不破坏昨天刚发布的配置项。那是同一个清理工作的更小、且非破坏性的版本——值得认真考虑,但不是阻断项。

风险: 无升级风险信号——Stage 1e 的回撤历史路径模式在本 diff 中未命中任何文件。

我会继续做代码审查,让接手的人能在一个地方看到完整信息;但真正决定这个 PR 的是上面的方向问题。🙏

Qwen Code · qwen3.8-max-2026-09-02

Reviewed at 60eeddc2cafd301b0ff11fa110dccffee1266d66 · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

🩺 serve daemon A/B

Built the PR base vs this PR head 60eeddc, drove a fixed endpoint set against each, and diffed the JSON responses. Only fields that changed are shown.

health-deep-with-session

field PR base (before) this PR (after)
activeWorkStaleMs 10 8

Qwen Code · serve A/B

@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

Code review

Independent proposal first. From the title and the "Why it's needed" section alone, before reading the diff, I would have done three things: opened an issue recording the decision to remove a setting that shipped in v0.23.2 one day earlier; deleted the rewriting layer first (displayTextOffset, messagePrefixText, Feishu's stripLeadingMentionNames, QQ's offset plumbing) while keeping messagePrefix as a pure drop-non-matching filter, since the rewriting is what creates the /review /new collision and the attacker-controlled-nick hazard; and, if removing wholesale anyway, kept a one-release deprecation that logs a warning when the key is still present. Comparing that to the actual diff: the PR does the wholesale removal, executes it better than I would have, and skips the issue and the deprecation. The execution gap is zero; the process gap is the two things I named.

No Critical blockers found. I looked hard for the three failure modes a ten-adapter removal invites, and none are present:

  • Completeness. Rather than eyeball it, I enumerated every file on main referencing any prefix symbol (messagePrefix, startsWithMessagePrefix, stripMessagePrefix, applyMessagePrefix, bypassMessagePrefix, messagePrefixText, displayTextOffset, configuredMessagePrefix, wasMessagePrefixRejected, prefixedCommand): exactly 32. This PR changes exactly those 32 files and nothing else. No orphaned references, no dead exports, no drive-by churn.
  • Behaviour drift on the no-prefix path — the path every existing user who never configured this is on. I traced each adapter against main and all are identical when no prefix is set: QQ's rawCommandText/displayText/text reduce to the same expressions; Telegram's four buildEnvelope call sites correctly shift after the allowRegisteredCommandBypass positional is dropped (this is exactly where a syntheticText value would have landed in the wrong slot, and it doesn't); Telegram's envelope.text = msg.caption || '' and promptText simplify to what the old !configuredMessagePrefix() branch already produced; DWS's two group-history callers now omit the third createImEnvelope argument, which defaults to message.content.trim() — the same value stripMessagePrefix(x, undefined) returned; GitHub's comment aggregation reduces flatMapmap with the same .trim().
  • Dead symbols left behind (the noUnusedLocals / ESLint trap). Every local whose only reader was deleted went with it: commandText in QQ's handleC2C, isMentionTrigger in GitLab, optionalPlainStringField in config-utils.ts (exactly one caller, both removed together). And the ones that look prefix-related but aren't were correctly kept: alreadyPrefixed still has live readers in ChannelBase.ts (sender attribution) and GithubAdapter.ts; syntheticText still drives quoted-group-history exclusion; stripBotMention still has two other call sites; rawMessageId still has a dozen.

One thing I verified rather than assumed — the config compatibility claim. The PR body says an unchanged legacy key may remain but can't be added or changed. That's true, and it falls out of existing machinery rather than new code: parseChannelConfig reads known fields individually and never rejects unknown keys, so a v0.23.2 settings.json carrying messagePrefix still loads and the key is simply ignored. On the managed path, dropping messagePrefix from the shared-string list in channel-settings-store.ts routes it to the pre-existing assertPreservedUnknownField, which returns silently only when previous owns the key with a deep-equal value and otherwise throws not manageable. No migration code needed, none added. That's the right way to do it.

Suggestions, not blockers:

  1. The ignored key is silent. A user who set messagePrefix: "/review" in v0.23.2 upgrades and their channel starts answering everything, with nothing in the log telling them their filter stopped working. A one-release stderr warning on an unrecognized-but-known-formerly-supported key would cost a few lines and convert a confusing behaviour change into a legible one. This is the same point as the Stage 1 direction question, restated as something you could actually do in this diff.
  2. Test coverage for the retained behaviours survives the rewrite, which is worth saying because it usually doesn't: the WeChat synthetic-marker case was re-anchored on createChannel() instead of a prefixed channel, the "rejects before preparing media" case swapped its trigger from prefix mismatch to a disallowed group, and the GitLab todo case now asserts on real dispatched text rather than on a bypassMessagePrefix flag. Those are equal-or-stronger assertions, not weakened ones. The channel-registry flips to not.toContainEqual are the correct direction for a removal.
  3. No design doc, which AGENTS.md asks for on non-trivial work or design decisions. Noting it because a breaking removal of a shipped feature is squarely both — but this is a Suggestion under the project's own review rules, not a Critical.
Files changed (17 of 32 shown — production and docs; the other 15 are their collocated tests)
File What changed
docs/users/features/channels/overview.md Drops the settings-table row and both prose paragraphs, including the one documenting that the prefix is checked before pairing
packages/channels/base/src/message-prefix.ts Deleted outright, 95 lines
packages/channels/base/src/index.ts Drops the two public re-exports — this is the package-API break
packages/channels/base/src/types.ts Removes ChannelConfig.messagePrefix and three Envelope fields; keeps syntheticText with the prefix rationale reworded out
packages/channels/base/src/ChannelBase.ts Removes the prefix fields, both WeakSets, the constructor type-check, three protected accessors and the preflight gate block; unwraps roughly thirty command strings to bare slash form
packages/channels/dws/src/dws-channel.ts Removes dwsMessagePrefix, the two group-history strip branches and three bypass markers; callers now use the envelope factory's own text default
packages/channels/feishu/src/FeishuAdapter.ts Deletes stripLeadingMentionNames and the mentionNames plumbing that only fed it; keeps userAuthoredText driving syntheticText
packages/channels/github/src/GithubAdapter.ts Comment aggregation no longer strips a prefix; bypass guard dropped from the envelope type predicate and two producers
packages/channels/gitlab/src/GitlabAdapter.ts Drops the branch that set the bypass flag on non-mention todos
packages/channels/qqbot/src/QQChannel.ts The deepest adapter change: collapses the prefix-aware command/payload/offset derivation in both the shared prepare path and C2C
packages/channels/telegram/src/TelegramAdapter.ts Removes the command-menu bypass parameter and its computation; simplifies four media-caption paths; start message always shows bare commands
packages/channels/wecom/src/WeComAdapter.ts Removes the prefix-rejected seenMessages bookkeeping
packages/channels/weixin/src/WeixinAdapter.ts Comment-only: drops the prefix rationale above the retained synthetic marker
packages/cli/src/commands/channel/channel-registry.ts Removes the field descriptor, so the Web Shell stops offering Message Prefix
packages/cli/src/commands/channel/config-utils.ts Removes the parse and its now-callerless helper; unknown keys remain ignored, which is what keeps legacy configs loading
packages/cli/src/serve/channel-settings-store.ts Drops the key from the managed shared-string list, routing it to the existing preserved-unknown-field tolerance
…and 16 test files Re-anchored on unprefixed configs rather than deleted; see Suggestion 2

Testing evidence

This is an unattended CI run, so per the gate's rules I did not build or execute anything from this PR — the review above is static, read from the diff and from main in an isolated worktree. The evidence below is the PR's own CI, read through the API for the reviewed commit. No check has failed. The check that matters most for this diff has not finished yet.

Final CI results for 60eeddc (auto-updated by the triage finalize job after CI completed):

Check Conclusion
Classify PR ✅ success
Desktop Shell (ubuntu-22.04) ✅ success
Desktop Shell (windows-2022) ✅ success
Integration Tests (no-AK, No Sandbox) ✅ success
Lint & Static (ubuntu-latest, Node 22.x) ✅ success
macos-latest / Java 21 ✅ success
OpenTUI no-flicker gate ✅ success
Real daemon E2E / Java 11 ✅ success
Serve A/B (ubuntu-latest, Node 22.x) ✅ success
Test (ubuntu-latest, Node 22.x) ✅ success
TUI parity snapshots (ink vs opentui) ✅ success
ubuntu-latest / Java 11 ✅ success
ubuntu-latest / Java 17 ✅ success
ubuntu-latest / Java 21 ✅ success
web-shell E2E Smoke (ubuntu-latest, Node 22.x) ✅ success
windows-latest / Java 21 ✅ success

One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。

Reading that honestly: Lint & Static passing is a real signal for a removal PR — it means ESLint and Prettier are clean across the changed files, which is where an orphaned import or an unused local would have surfaced. But Test (ubuntu-latest, Node 22.x) is the check that actually runs the channels and CLI unit suites this PR rewrites, and it is still in progress. I am not going to report that as passing. The macOS and Windows unit jobs are skipped, so ubuntu is the only platform signal this PR will get from CI. verify and tmux-testing are skipped because nobody has triggered them — that is the gap the next section names.

Not verified: runtime dispatch behaviour on any live channel. I did not execute PR code, the ubuntu unit suite had not landed at review time, and no sandboxed lane has been run.

The author's own evidence, attributed as such and not counted as mine: a comment on this PR reports a 34-case local script against the shared channel runtime (22 passed / 12 expected failures on the upstream baseline, 34/34 on the new build), plus 658 DingTalk and 188 CLI tests, full build, bundle and typecheck. That is a serious and specific write-up, and the baseline-versus-new pairing is the right shape for a removal. But it is the author's claim about a local run I cannot inspect or re-run, and one part of it is not externally checkable at all — the statement that nine channel trees are "byte-for-byte identical to the previously validated trees" refers to the author's own earlier branches, not to main. Treat it as supporting, not as CI.

Sandboxed verification would settle what neither my static read nor the pending ubuntu suite can: @qwen-code /verify — the load-bearing claim here is that dispatch is unchanged for every user who never configured a prefix, across ten adapters, and a green suite does not pin that (it would pass identically with the removal half-done). An A/B against the base build is exactly the proof. And @qwen-code /tmux for the two surfaces a human would actually look at: that channel settings no longer offer a Message Prefix field while a stored legacy key still saves cleanly alongside other edits, and that /help in a channel now renders bare /clear, /approve, /session commands. You have write access, so both lanes are available directly rather than needing a sponsored run.

中文说明

代码审查

先说我自己的方案。 只看标题和「Why it's needed」、在读 diff 之前,我会做三件事:先开一个 issue 记录「移除一个一天前才随 v0.23.2 发布的配置项」这个决定;先只删改写正文那一层(displayTextOffsetmessagePrefixText、飞书的 stripLeadingMentionNames、QQ 的 offset 管线),把 messagePrefix 保留成纯粹的「不匹配即丢弃」过滤器,因为正是改写造成了 /review /new 冲突和「昵称可被攻击者控制」的隐患;如果还是要整体移除,就保留一个版本的废弃过渡,在键仍存在时打一条警告日志。与实际 diff 对比:本 PR 做的是整体移除,执行得比我会做的更好,但缺了 issue 和废弃过渡这两件事。执行层面没有差距,流程层面差的就是我点出的这两项。

未发现 Critical 阻断问题。 我重点找了「十个渠道适配器同时移除」最容易出的三类问题,都不存在:

  • 完整性。 我没有靠肉眼,而是枚举了 main 上引用任何前缀相关符号的全部文件(messagePrefixstartsWithMessagePrefixstripMessagePrefixapplyMessagePrefixbypassMessagePrefixmessagePrefixTextdisplayTextOffsetconfiguredMessagePrefixwasMessagePrefixRejectedprefixedCommand):恰好 32 个。本 PR 改动的正好是这 32 个文件,没有多改任何一个。没有残留引用、没有死导出、没有夹带改动。
  • 未配置前缀路径上的行为漂移——也就是所有从未配置过该选项的现存用户所走的路径。我逐个适配器对照 main 追踪,在未配置前缀时全部一致:QQ 的 rawCommandText/displayText/text 归约为完全相同的表达式;Telegram 四处 buildEnvelope 调用在去掉 allowRegisteredCommandBypass 这个位置参数后位移正确(这正是 syntheticText 最容易落错参数位的地方,而它没有);Telegram 的 envelope.text = msg.caption || ''promptText 化简后与旧代码 !configuredMessagePrefix() 分支的产出相同;DWS 两处群历史调用现在省略 createImEnvelope 的第三个实参,而该参数默认值 message.content.trim() 正是 stripMessagePrefix(x, undefined) 原本返回的值;GitHub 的评论聚合由 flatMap 归约为 map.trim() 不变。
  • 残留死符号noUnusedLocals / ESLint 陷阱)。唯一读取方被删掉的局部变量都一并删了:QQ handleC2C 里的 commandText、GitLab 的 isMentionTriggerconfig-utils.ts 里的 optionalPlainStringField(恰好只有一个调用方,两者同时移除)。而那些看起来与前缀相关、实际不是的符号被正确保留:alreadyPrefixedChannelBase.ts(发送者归属)和 GithubAdapter.ts 中仍有活跃读取方;syntheticText 仍在驱动「引用群历史排除」;stripBotMention 还有另外两处调用;rawMessageId 还有十几处使用。

有一处我是去核实而不是想当然的——配置兼容性主张。 PR 描述说「旧键原样保留可以,但不能新增或修改」。这是成立的,而且它是靠既有机制自然达成的,没有新增代码:parseChannelConfig 逐个读取已知字段、从不拒绝未知键,所以带着 messagePrefix 的 v0.23.2 settings.json 仍能正常加载,该键只是被忽略。托管路径上,把 messagePrefixchannel-settings-store.ts 的 shared-string 列表里去掉后,它会落到既有的 assertPreservedUnknownField——只有当 previous 拥有该键且值深度相等时才静默通过,否则抛 not manageable。不需要迁移代码,也没有加。这是正确的做法。

建议(非阻断):

  1. 忽略该键是静默的。在 v0.23.2 里设了 messagePrefix: "/review" 的用户升级后,渠道会开始回应所有消息,而日志里没有任何东西告诉他过滤器已失效。在「曾支持、现已移除」的键上打一条 stderr 警告、保留一个版本,只需几行代码,就能把一次令人困惑的行为变化变成一次可读的行为变化。这与 Stage 1 的方向问题是同一点,只是改写成你在本 diff 里真能做的事。
  2. 保留行为的测试覆盖在改写后存活了下来——这点值得说,因为通常不会:微信的 synthetic 标记用例改为锚定 createChannel() 而非带前缀的渠道;「在准备媒体前拒绝」用例把触发条件从前缀不匹配换成不允许的群聊;GitLab 的 todo 用例改为断言真实投递的文本,而不是断言一个 bypassMessagePrefix 标志。这些都是等强或更强的断言,不是被削弱的断言。channel-registry 翻转为 not.toContainEqual 对于一次移除来说是正确方向。
  3. 没有设计文档,而 AGENTS.md 要求非平凡改动或涉及设计决策时提供。之所以点出,是因为「破坏性移除一个已发布功能」两者都占——但按项目自己的 review 规则,这是 Suggestion 而非 Critical。

测试证据

这是一次无人值守的 CI 运行,因此按 gate 规则我没有构建或执行本 PR 的任何代码——上面的审查是静态的,读的是 diff 以及隔离 worktree 里的 main。下面的证据是本 PR 自己的 CI,通过 API 针对被审 commit 读取。没有任何 check 失败。 但对这个 diff 最关键的那个 check 还没跑完。

如实解读:Lint & Static 通过对一个移除类 PR 是真实信号——它意味着改动文件的 ESLint 与 Prettier 干净,而孤立的 import 或未使用的局部变量正是会在这里暴露。但是,Test (ubuntu-latest, Node 22.x) 才是真正运行本 PR 重写的 channels 与 CLI 单测的 check,它仍在进行中。 我不会把它报成通过。macOS 与 Windows 的单测 job 被跳过,所以 ubuntu 是本 PR 能从 CI 拿到的唯一平台信号。verifytmux-testing 被跳过是因为还没人触发——那正是下一节点名的缺口。

未验证: 任何真实渠道上的运行时分发行为。我没有执行 PR 代码,审查时 ubuntu 单测尚未出结果,也没有跑过任何沙箱验证通道。

作者自述的证据,明确归属于作者、不计入我的证据: 本 PR 上的一条评论报告了一个针对共享渠道运行时的 34 例本地脚本(上游基线 22 通过 / 12 例预期失败,新构建 34/34 通过),外加 658 个 DingTalk 测试、188 个 CLI 测试,以及完整的 build、bundle 和 typecheck。这是一份认真且具体的说明,基线与新版本配对也正是移除类改动该有的形状。但它是我无法检查也无法重跑的本地运行的作者主张,其中有一处完全无法外部核验——「九个渠道树与先前已验证的树逐字节一致」指的是作者自己先前的分支,而不是 main。请把它当作辅助材料,而非 CI。

沙箱验证能解决我的静态阅读和尚未出结果的 ubuntu 单测都解决不了的问题:@qwen-code /verify——这里承重的论断是「对所有从未配置前缀的用户,十个渠道的分发行为完全不变」,而一套绿色的测试并不能钉住这一点(移除只做一半它同样会通过)。与 base 构建做 A/B 正是所需的证明。另外 @qwen-code /tmux 用于两个真正需要人眼看的界面:渠道设置不再提供 Message Prefix 字段、同时已存储的旧键仍能与其它编辑一起正常保存;以及渠道内 /help 现在渲染的是裸的 /clear/approve/session 命令。你具备 write 权限,因此这两个通道可以直接触发,不需要 sponsored run。

Qwen Code · qwen3.8-max-2026-09-02

Reviewed at 60eeddc2cafd301b0ff11fa110dccffee1266d66 · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 3/5 — the code review came back clean and I could not break it; the score is capped by policy, because removing a setting that shipped in stable v0.23.2 the day before is a public-contract call that belongs to a maintainer, not to this gate.

Stepping back from the diff, here's what I actually think.

The execution is the easy part to judge and it holds up. I went in expecting to find the usual damage a ten-adapter removal leaves behind — an orphaned export, a positional argument shifted into the wrong slot, a local whose only reader was deleted, one adapter quietly left behind. I found none of it, and I checked by enumeration rather than impression: the files on main that reference any prefix symbol number exactly 32, and this PR touches exactly those 32. When a sweep that wide lands that precisely, somebody was methodical about it. Six months from now I'd thank the author for 1,644 fewer lines of per-adapter special-casing, not curse them.

What I can't settle is whether the removal is right, and the reason I can't settle it changed while I was looking. This started as "no linked issue, breaking change, no deprecation window" — all still true. Then I checked what else you have open, and there are 14 PRs, with a whole channels stack in flight: #11570 removing DingTalk response aggregation, #11566 fixing DingTalk mention text (which this PR's description says must merge after it), #11561 adding a shared output policy, #11429 enhancing DingTalk status cards. Read alone, this PR looks like deleting a five-day-old feature for no recorded reason. Read as one move in that stack, it looks like an inbound-routing boundary being cleared out to make room for a different design. I can see the shape of that from here but I can't see the intent, and the difference between those two readings is the entire decision. That is not something a gate should resolve by inference, and it's the honest reason this is a 3 and not a 4.

So, concretely, for whoever picks this up — three questions, and only the first one is about code:

  1. Is the redesign real, and does this belong in it? If feat(channels): add shared output policy with DingTalk support #11561's output policy is where routing is moving, say so here and link the stack. That single sentence converts this from an unexplained breaking removal into an obvious step, and it's the thing most missing from the PR as filed.
  2. What do v0.23.2 users get? I confirmed the compatibility mechanics are sound — a legacy key still loads, still saves unchanged, and can't be added or modified, all through existing machinery with no migration code. That part needs no work. What's absent is any signal: the key goes quiet, and someone's channel starts answering messages it used to drop, including the pairing-reply noise in busy groups that the docs paragraph you deleted says the prefix existed to suppress. A one-release warning is a few lines and turns a confusing upgrade into a legible one.
  3. Should there be an issue and a design doc? feat(channels): add configurable inbound message prefix #10816 was filed to add this and was triaged as aligned with product direction five days ago. Removing it deserves the same paper trail, if only so the reversal is legible to anyone who wasn't in the room. AGENTS.md asks for a design doc on non-trivial work; this is 32 files and a breaking change to a released surface.

On the evidence: Lint & Static is green, which for a removal PR is where an orphaned import would have surfaced, and nothing has failed. But Test (ubuntu-latest, Node 22.x) — the job that actually runs the channels and CLI suites this PR rewrites — was still in flight when I wrote this, and macOS and Windows unit jobs are skipped, so ubuntu is the only platform signal coming. I'm not reporting that suite as passing. Your own 34-case write-up on this PR is specific and the right shape for a removal, and I've attributed it as your claim in Stage 2 rather than counting it as CI.

Two things I'm deliberately not doing, so nobody waits on the wrong signal:

  • I'm not requesting changes. I found no defect — no correctness bug, no regression on the no-prefix path, no dead code, no scope creep. Asking for rework on a clean diff because I'd have filed an issue first would be the gate inventing a blocking rule it doesn't have.
  • I'm not approving, and I have not left an approve-on-green instruction. Approval here is deferred to a human, not to CI — when the ubuntu suite lands green, nothing will auto-approve this, and that's intentional. The fork-refactor guardrail doesn't apply (this is feat), so the only thing holding approval is the public-contract escalation above.

One housekeeping note: I tried to resolve an accountable maintainer deterministically and got nothing — QWEN_MAINTAINER_HANDLE is unset, the PR carries no labels so the owner-map resolver matched no area, and there are no human reviews yet to fall back on. The workflow says don't guess a login, so I'm not @mentioning anyone. Adding an area label would let assignment resolve on a re-run. Since you're a maintainer with write access yourself, the practical ask is just to pull in whoever owns the channels direction for the stack question above.

中文说明

Confidence: 3/5 —— 代码审查结论是干净的,我没能找出问题;分数被政策封顶,因为移除一个前一天才随 stable v0.23.2 发布的配置项属于公共契约决策,该由维护者定夺,而不是由这个 gate 定夺。

从 diff 里退一步说,我真实的看法是这样的。

执行层面最容易判断,而它站得住。我原本预期会看到「十个适配器同时移除」通常会留下的破坏——孤立的导出、位移到错误参数位的实参、唯一读取方被删掉的局部变量、某个被悄悄漏掉的适配器。这些一个都没有,而且我是靠枚举而不是印象核对的:main 上引用任何前缀相关符号的文件恰好 32 个,本 PR 改动的正好是这 32 个。这么宽的一次清扫能落得这么精确,说明作者做事是有方法的。六个月后回看,我会感谢作者少了 1,644 行按渠道特化的代码,而不是骂他。

我无法定论的是这次移除是否正确,而且我无法定论的原因在查看过程中发生了变化。一开始是「没有关联 issue、破坏性变更、没有废弃过渡期」——这些依然成立。然后我去看了你还有哪些 PR 在开着:有 14 个,其中一整套 channels 的工作正在推进——#11570 移除 DingTalk 的响应聚合、#11566 修复 DingTalk mention 文本(本 PR 描述说它必须在此之后合并)、#11561 新增共享的 output policy、#11429 增强 DingTalk 状态卡片。单独看,这个 PR 像是在没有任何记录理由的情况下删掉一个五天前才加的功能;放到那一摞里看,它像是在为另一套设计腾地方而清掉一条入站路由边界。我在这里能看出那个形状,但看不出意图,而这两种解读之间的差别就是整个决策本身。这不是一个 gate 应该靠推断去解决的问题,也是它诚实地停在 3 分而不是 4 分的原因。

所以,具体给接手的人三个问题,其中只有第一个是关于代码的:

  1. 那个重设计是真的吗,这次移除属于它吗? 如果 feat(channels): add shared output policy with DingTalk support #11561 的 output policy 就是路由要迁往的方向,请在这里说明并把那一摞 PR 串起来。这一句话就能把本 PR 从「无解释的破坏性移除」变成「显然的一步」,而这正是当前 PR 提交形态里最缺的东西。
  2. v0.23.2 的用户会得到什么? 我确认了兼容性机制是可靠的——旧键仍能加载、仍能原样保存、且不能被新增或修改,全部经由既有机制完成,没有新增迁移代码。这部分不需要再做工作。缺的是任何提示:该键会静默失效,某人的渠道会开始回应它过去会丢弃的消息,其中包括你删掉的那段文档所说、前缀存在正是为了抑制的活跃群聊配对回复噪音。保留一个版本的警告只需几行,就能把一次令人困惑的升级变成一次可读的升级。
  3. 是否应该有一个 issue 和设计文档? feat(channels): add configurable inbound message prefix #10816 是为新增此功能而提的,五天前被 triage 判定为与产品方向一致。移除它值得有同样的纸面记录,哪怕只是为了让这次反转对不在场的人来说是可读的。AGENTS.md 要求非平凡改动提供设计文档;而这是 32 个文件、对已发布界面的破坏性变更。

关于证据:Lint & Static 是绿的——对一个移除类 PR 来说,孤立的 import 正是会在那里暴露——且没有任何 check 失败。但 Test (ubuntu-latest, Node 22.x),也就是真正运行本 PR 重写的 channels 与 CLI 测试套件的那个 job,在我写下这段时仍在进行中;而 macOS 与 Windows 的单测 job 被跳过,所以 ubuntu 是唯一会来的平台信号。我不会把那个套件报成通过。你自己在 PR 上那份 34 例的说明很具体,形状也正是移除类改动该有的,我在 Stage 2 里把它归属于你的主张,而没有当作 CI 计入。

有两件事我是刻意不做的,免得有人等错信号:

  • 我没有 request changes。 我没发现缺陷——没有正确性 bug、未配置前缀路径上没有回归、没有死代码、没有范围蔓延。仅仅因为「我会先开个 issue」就要求一份干净的 diff 返工,那是 gate 在发明它并不拥有的阻断规则。
  • 我没有批准,也没有留下 approve-on-green 指令。 这里的批准是交给人,而不是交给 CI——ubuntu 套件跑绿之后,不会有任何东西自动批准这个 PR,这是有意的。fork-refactor 护栏并不适用(这是 feat),所以唯一挡住批准的就是上面那条公共契约升级。

一个流程性说明:我尝试用确定性方式解析出一位负责的维护者,但没有得到结果——QWEN_MAINTAINER_HANDLE 未设置,PR 没有任何标签所以 owner-map 解析器未匹配到任何区域,也还没有人类 review 可供兜底。工作流规定不要猜一个登录名,所以我没有 @ 任何人。补一个区域标签可以让重新运行时的指派解析成功。由于你自己就是有 write 权限的维护者,实际的请求只是:请把负责 channels 方向的人拉进来,回答上面那个「一摞 PR」的问题。

Qwen Code · qwen3.8-max-2026-09-02

Reviewed at 60eeddc2cafd301b0ff11fa110dccffee1266d66 · re-run with @qwen-code /triage

@qqqys
qqqys enabled auto-merge September 10, 2026 13:31

@qwen-code-dev-bot qwen-code-dev-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved at head 60eeddc2.

Required CI is green — Test (ubuntu-latest, Node 22.x), Lint & Static, Integration Tests (no-AK, No Sandbox) and web-shell E2E Smoke completed successfully; only review-pr (the reviewer's own job) is still running.

This is a breaking removal across eight packages, so the bar I held it to was "nothing left hanging" rather than "the hot paths look fine":

  • Every removed name has zero references at this head: messagePrefix, message-prefix, prefixedCommand, configuredMessagePrefix, bypassMessagePrefix, messagePrefixText, displayTextOffset, startsWithMessagePrefix/stripMessagePrefix. The shared module, its 240-line test file, both base index.ts exports and the three option fields in ChannelOptions/adapter inputs are gone together, and per-adapter deletions (telegram, feishu, qqbot, github, gitlab, wecom, weixin, dws) removed the call sites rather than leaving them fed by defaults.
  • The two WeakSet bookkeepers in ChannelBase existed only to remember which envelopes had been prefix-checked or prefix-rejected, so deleting them takes no other behavior with it; the comment that survived in their place still records the part that matters independently of the prefix — a synthetic media placeholder is never recorded as quoted group history, so it cannot come back as if a member had typed it.
  • Legacy settings are handled honestly rather than swept under a migration: a stored messagePrefix is now just unknown configuration data, which config-utils tolerates by iterating declared fields, and there are three tests pinning the compatibility surface — the base preserving an unchanged legacy key, config-utils accepting it as unknown data, and channel-settings-store both preserving it across an unrelated save and rejecting a managed write of it (Channel field "messagePrefix" is not manageable.). No config migration is claimed, and none is needed for that to be true.
  • The help and permission strings are the part I checked most closely, because a wrong command in a user-facing hint is a silent usability break. Every prefixedCommand(...) wrapper became the bare slash command, so /help, /clear confirm, /approve [request-id], /deny [request-id], /session use <name> and the multi-pending-request guidance all now name something the runtime actually accepts — and the removed validation (field "messagePrefix" must be a string) went with the field it validated.

The breaking consequence is stated where it should be, including the security-relevant half: a deployment that used the prefix to narrow what reaches the agent now admits everything its remaining sender/group/mention/pairing policies allow. feat(channels)! is the right tag for that.

No new Critical found. Two non-blocking notes: docs/users/features/channels/overview.md deletes the whole prefix section without a "previously configured keys are ignored now" line for operators who still have one set, which pairs with the same choice in the sibling cleanup; and this PR intentionally excludes the DingTalk mention-body parser fix from #11566, so whichever of the two merges second should re-check that messagePrefixText-shaped adapter hook is really gone rather than re-introduced by the other.

@yiliang114 yiliang114 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the PR-owned surface at head 60eeddc2 (32 files, +339/-1644). No blocking finding. Approving, with three non-blocking notes — the second one is a control-integrity delta worth accepting consciously rather than by omission.

A ! PR that removes a filter from every inbound-message path at once deserves one question above all others: was that filter an authorization control? It was not. applyMessagePrefix short-circuited return true when the prefix was unset, and the config key defaulted to undefined, so out of the box nothing was filtered. What is being withdrawn is a traffic-narrowing control, and the PR body says so. The gates that actually decide whether a message reaches the model are untouched and still run in the same order: groupGate.check (ChannelBase.ts:6150, now the first statement of preflightInbound), dmGate.check (:6187), sender/pairing policy, and the mention_required group handling with recordPendingGroupHistory.

The removal is uniform — no adapter kept a private copy. Base had the shared helper plus four adapter-private extensions (dws-channel.ts with its own dwsMessagePrefix field and three stripMessagePrefix call sites; FeishuAdapter.ts with stripLeadingMentionNames(); GithubAdapter.ts stripping inside comment aggregation; QQChannel.ts deriving messagePrefixText/displayTextOffset). At head, git grep for messagePrefix, MessagePrefix, prefixedCommand, displayTextOffset and message_prefix across packages/ returns zero hits, versus 8 non-test hits at base. The surviving alreadyPrefixed references (types.ts:145, ChannelBase.ts:2033/6808/6920, QQChannel.ts:2630/2718/2872, GithubAdapter.ts:388) are a different, still-live feature — the per-turn [senderName] attribution marker — correctly retained rather than half-removed. I also checked the two adapter simplifications that could have drifted behaviour and found them equivalent: dws documentNotification now parses from text instead of rawText, which are identical once stripping is gone; Telegram's msg.caption ternaries collapse to the no-prefix branch, which was already the default.

The managed settings path does push back, which is the part most likely to be missed in a removal: assertSharedField (channel-settings-store.ts:124-148) dropped 'messagePrefix' from its allowed list, so the key now falls through to Channel field "messagePrefix" is not manageable., and the rewritten tests pin both halves — an unchanged stored key round-trips untouched while adding or changing it rejects with channel_settings_invalid_config. channel-registry.ts removed the field descriptor and docs/users/features/channels/overview.md deleted the row and both explanatory paragraphs.

Test deletions are replacements, not holes. checks a prepared envelope once and rejects before preparation became rejects disallowed groups before preparing media and still calls handlePreparedInbound(rejected, prepare) twice before asserting prepare was not called, so rejection idempotence survives on the group-gate path; requires the prefix on a pairing first contact too became offers pairing on first contact with ordinary text; DingTalk's five prefix tests were rewritten to assert the surviving syntheticText contract rather than deleted. No surviving public or protected ChannelBase method lost its only test — the three removed helpers (prefixedCommand, configuredMessagePrefix, wasMessagePrefixRejected) went with their call sites.

Three notes

1. The raw settings.json path is silent where the managed path is not. parseChannelConfig no longer reads the key (config-utils.ts:513 removed optionalPlainStringField(name,'messagePrefix',…)), the ChannelConfig field is gone (types.ts:57), and nothing emits a notice. So an operator who set messagePrefix: "/review" to keep a bot quiet upgrades and gets no log line, no warning, no startup failure — and from that moment every message clearing the remaining gates dispatches a run, with the raw prefix now reaching the model verbatim. The PR's own new test proves the inertness: it.each(['hello','/review /new','@Qwen /review inspect this'])('preserves %s when an old config still contains messagePrefix', …) asserts bridge.prompt is called with the text unchanged and bridge.discardSession is not called, so /review /new is no longer rewritten into the local reset command. Blast radius is bounded by the group gate (the deleted test had to set groupPolicy: 'open' explicitly to let ambient chatter through), so this is not a flood — but it is a behaviour change with zero operator-visible signal on the one path that does not reject. A one-line stderr deprecation notice from parseChannelConfig when it discards the key would make the two paths consistent.

2. Deleting the prefix bypass also deleted Telegram's only @OtherBot guard. TelegramAdapter.ts buildEnvelope (head :496-558) lost the allowRegisteredCommandBypass parameter and the whole bypassMessagePrefix computation, and with it the block whose own comment read: "/cancel@OtherBot is addressed to a different bot. parseCommand strips the suffix, so without this check the bypass would let a command meant for someone else past the prefix gate and run it here — cancelling our own request, for instance." In a Telegram group hosting two bots, /cancel@OtherBot can now cancel our running request. To be fair about scope: the bypass was inert whenever messagePrefix was unset, so this matches long-standing behaviour for every non-prefix deployment and is a declared consequence of the removal, not a new defect. But it is a control-integrity delta rather than mere extra noise, and the fix is cheap and strictly better than what was there — move the @OtherBot check into parseCommand/command dispatch, where it protects all configurations instead of only prefixed ones.

3. Two public exports of the published @qwen-code/channel-base package were removed (index.ts base lines 43-46: startsWithMessagePrefix, stripMessagePrefix), and the ! PR body's breaking-change note mentions only the config key. Nothing in the monorepo is affected — all five consumers (dws-channel.ts:13, FeishuAdapter.ts:14, GithubAdapter.ts:31, QQChannel.ts:22, TelegramAdapter.ts:15) had their imports removed in this PR — but an out-of-tree channel plugin importing either symbol fails to compile after upgrade with no migration note to point at. One line in the breaking-change section naming the removed exports would close it.

Cross-PR check

#11570 modifies packages/channels/dingtalk/src/DingtalkAdapter.test.ts too and shares this merge-base, so I checked the pair: git merge-tree --write-tree --name-only pr-11570-review pr-11571-review exits 0 with no conflicted-file section, and the two edit disjoint regions of that file. Landing order between them does not matter. The ordering constraint this PR states is its own and unrelated: it must merge before #11566.

CI at head: green apart from review-pr. No review threads. I did not run the suite locally — no node_modules in this checkout — and I did not read config-utils.test.ts (+10/-11), so I cannot confirm whether a test covers the real settings.json → parse → construct chain for a legacy key; the new ChannelBase.test.ts case constructs legacyConfig and passes it straight to createChannel, which exercises the constructor only.

中文说明

在 head 60eeddc2 上审了本 PR 自有的范围(32 个文件,+339/-1644)。没有阻塞项。 批准,附三条非阻塞说明——第二条是一个值得有意识接受、而不是靠遗漏接受的管控完整性变化。

一个在所有入站消息路径上一次性移除过滤器的 ! PR,最该回答的问题只有一个:那个过滤器是不是授权控制?不是。applyMessagePrefix 在前缀未设置时直接 return true,而该配置键默认 undefined,所以开箱状态下什么都没被过滤。被撤回的是一个收窄流量的控制,PR 正文也是这么说的。真正决定消息能否到达模型的门禁未被触碰、且顺序不变:groupGate.checkChannelBase.ts:6150,现在是 preflightInbound 的第一句)、dmGate.check:6187)、发送者/配对策略,以及带 recordPendingGroupHistorymention_required 群处理。

移除是统一的——没有 adapter 留私拷贝。 base 有共享 helper 加四处 adapter 私有扩展(dws-channel.ts 自己的 dwsMessagePrefix 字段与三处 stripMessagePrefixFeishuAdapter.tsstripLeadingMentionNames()GithubAdapter.ts 在评论聚合里剥离;QQChannel.ts 推导 messagePrefixText/displayTextOffset)。head 上对 messagePrefixMessagePrefixprefixedCommanddisplayTextOffsetmessage_prefixpackages/ 下 grep 零命中(base 有 8 处非测试命中)。存活的 alreadyPrefixed 引用(types.ts:145ChannelBase.ts:2033/6808/6920QQChannel.ts:2630/2718/2872GithubAdapter.ts:388)属于另一个仍然存活的特性——每回合的 [senderName] 归属标记——是正确保留而非半移除。我也查了两处可能造成行为漂移的 adapter 简化,结论是等价:dws 的 documentNotification 改为从 text 而非 rawText 解析,而剥离去掉后两者相同;Telegram 的 msg.caption 三元式收敛到无前缀分支,而那本来就是默认行为。

托管设置路径确实会拒绝,这是移除类改动最容易漏的一环:assertSharedFieldchannel-settings-store.ts:124-148)把 'messagePrefix' 从允许列表里去掉,所以该键现在落到 Channel field "messagePrefix" is not manageable.,且重写后的测试钉住了两半——已存键原样往返,新增或修改则以 channel_settings_invalid_config 拒绝。channel-registry.ts 移除了字段描述符,docs/users/features/channels/overview.md 删掉了表格行与两段说明。

测试删除是替换,不是留洞。 checks a prepared envelope once and rejects before preparation 变成 rejects disallowed groups before preparing media,仍然两次调用 handlePreparedInbound(rejected, prepare) 后断言 prepare 未被调用,所以拒绝幂等性在 group-gate 路径上存活;requires the prefix on a pairing first contact too 变成 offers pairing on first contact with ordinary text;DingTalk 的五个前缀测试被改写为断言存活的 syntheticText 契约而非删除。没有存活的 ChannelBase 公有/受保护方法失去唯一测试——被删的三个 helper(prefixedCommandconfiguredMessagePrefixwasMessagePrefixRejected)与其调用点一起消失。

三条说明

1. 裸 settings.json 路径是沉默的,而托管路径不是。 parseChannelConfig 不再读该键(config-utils.ts:513 移除了 optionalPlainStringField(name,'messagePrefix',…)),ChannelConfig 字段已删(types.ts:57),且没有任何提示。所以为了「让 bot 安静」而设过 messagePrefix: "/review" 的运维者升级后不会看到任何日志行、告警或启动失败——而从那一刻起,凡是过了剩余门禁的消息都会派发一次运行,且原始前缀会原样到达模型。本 PR 自己的新测试证明了这种惰性:it.each(['hello','/review /new','@Qwen /review inspect this'])('preserves %s when an old config still contains messagePrefix', …) 断言 bridge.prompt 收到未改动的文本、且 bridge.discardSession 未被调用,所以 /review /new 不再被改写成本地 reset 命令。影响范围被 group gate 限住(被删的那个测试必须显式设 groupPolicy: 'open' 才能放进环境噪声),所以不是洪水——但这是一次在唯一不会拒绝的路径上、零运维可见信号的行为变化。让 parseChannelConfig 在丢弃该键时输出一行 stderr 弃用提示,两条路径就一致了。

2. 删掉前缀 bypass 也删掉了 Telegram 唯一的 @OtherBot 守卫。 TelegramAdapter.tsbuildEnvelope(head :496-558)失去了 allowRegisteredCommandBypass 参数与整段 bypassMessagePrefix 计算,连同那段自带注释的代码:/cancel@OtherBot 是发给另一个 bot 的。parseCommand 会剥掉后缀,所以没有这道检查,bypass 会让本意给别人的命令越过前缀门禁并在这里执行——比如取消我们自己的请求。」 在同时挂了两个 bot 的 Telegram 群里,/cancel@OtherBot 现在能取消我们正在跑的请求。就范围而言要说公道话:bypass 在 messagePrefix 未设置时本来就是惰性的,所以这与所有无前缀部署的长期行为一致,是移除的既定后果而非新缺陷。但它是管控完整性的变化,不只是多了些噪声,而且修法便宜且严格优于原状——把 @OtherBot 检查移进 parseCommand/命令派发,它就能保护所有配置而不只是设了前缀的那些。

3. 已发布的 @qwen-code/channel-base 包有两个公开导出被移除index.ts base 第 43-46 行:startsWithMessagePrefixstripMessagePrefix),而 ! PR 正文的破坏性变更说明只提到配置键。monorepo 内不受影响——五个消费者(dws-channel.ts:13FeishuAdapter.ts:14GithubAdapter.ts:31QQChannel.ts:22TelegramAdapter.ts:15)的 import 都在本 PR 里删掉了——但一个 import 了这两个符号的树外 channel 插件升级后会编译失败,且没有迁移说明可指。在破坏性变更那节点名这两个被移除的导出即可。

跨 PR 检查

#11570 也改 packages/channels/dingtalk/src/DingtalkAdapter.test.ts 且共享同一 merge-base,所以我查了这一对:git merge-tree --write-tree --name-only pr-11570-review pr-11571-review 退出 0 且无冲突文件段,两者改的是该文件的不相交区域。两者之间的合并顺序无所谓。 本 PR 自己声明的顺序约束与之无关:它必须先于 #11566 合并。

head 上的 CI:除 review-pr 外全绿。无 review 线程。我没有在本地跑套件——这个 checkout 没有 node_modules——也没有读 config-utils.test.ts(+10/-11),所以无法确认是否有测试覆盖「真实 settings.json → parse → construct」链路上的遗留键;新的 ChannelBase.test.ts 用例是构造 legacyConfig 后直接传给 createChannel,只锻炼了构造函数。

@qqqys
qqqys added this pull request to the merge queue Sep 10, 2026
Merged via the queue into QwenLM:main with commit b3ebe1e Sep 10, 2026
73 of 74 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants