feat(group): 支持普通群负责人设置与跨部署校验 - #1319
Conversation
|
你好!这个 PR 的评审群已自动创建:pr 1319 支持普通群负责人设置 由于你暂时不在自动拉群名单里,未能直接把你拉进群。如果方便的话,请把你的 GitHub 账号和飞书信息补进作者名单文档,补好后后续复审会自动拉你进群。 这是自动流程,如有不便请见谅! |
|
您发给我的信件已经收到,非常感谢!--------------------------------------------Yang shan 祝您愉快!
|
|
你好,感谢这个 PR — 设计上有不少值得肯定的地方(先落本地意图再写远端、写后读回确认而不把 我在最新 🔴 建议合并前修复:打红了 2 个主干既有单测(必需
|
| 分支 | 结果 |
|---|---|
origin/master (0aba0fddd) |
Tests 320 passed (320) rc=0 |
| 本 PR rebase 后 | Tests 2 failed | 318 passed (320) rc=1 |
第二条是仓库刻意维护的契约:test/command-handler.test.ts:2877 遍历 [...DAEMON_COMMANDS, ...] 要求每一条都能在 /help 文案里找到,也就是说进了 DAEMON_COMMANDS 就必须在 /help 里可发现。目前 /manager 只写进了 docs-site,用户在飞书里发 /help 看不到它。
还有一点值得留意:/manager 是唯一被放进 DAEMON_COMMANDS 的 ingress 拦截命令 —— 同类的 /mention-mode、/reply-mode、/substitute、/introduce、/grant、/revoke、/invite 都不在该集合里。而 handleCommand 的 switch 里没有 case '/manager'(也没有 default:),所以万一有消息在 ingress 拦截之外走到 daemon 路由(我实测的一条:open 模式 bot 下 peer bot 发 @bot /manager set 会绕过 tryHandleManagerCommand 的真人路径,落到 handleThreadReply),/manager 会命中 DAEMON_COMMANDS 分支、先 createSession 建出一个 worker:null 的会话,然后在 switch 里静默无匹配返回。两个副作用:dashboard 里多一个幽灵会话;canTalkDaemonCommands 现在也接受 /manager(实测可配进去),而这条命令的权限其实由它自己的管理员门把守。
两个修法我都实测过:
- (A) 从
DAEMON_COMMANDS移除 —— 与全部同类命令一致,但会打红你自己新加的用例(normalizePassthroughCommand('/manager')期望为null,即防 CLI 透传抢占),所以单独做这一步不够:Tests 1 failed | 348 passed。 - (B) 保留该条目 + 补齐它带来的两个契约 ✅ 推荐 —— 加
help.manager文案(src/i18n/zh.ts/en.ts)+ 在command-handler.ts的 help 列表里加一行t('help.manager', …)+ 把 size 断言改成 39。实测Tests 349 passed (349)rc=0、tsc --noEmitrc=0。这样/help可发现性也顺带解决了。
若走 (B),建议同时考虑给 handleCommand 补一个 case '/manager'(回一句"请在普通群顶层 @ 我并使用 /manager set|clear|status"),把上面那条幽灵会话路径也堵掉。
🟠 非阻断
N1|群名后缀会累积,且 clear 再也恢复不回原名。 changeChatManager 的 set 分支基于 remote.name 拼后缀,而幂等短路只看远端 marker 在不在。如果有人在飞书 UI 里把群描述里的 [botmux:manager=…] 那行删掉(描述是人可编辑的,PR 本身也支持"保留人工修改"),再 set 一次就会二次追加:
round1 set → "Project · Manager"
round2 set → "Project · Manager · Manager"
round3 set → "Project · Manager · Manager · Manager"
最后 clear → 仍是 "Project · Manager · Manager · Manager"(原名 "Project" 丢了)
原因是第二次 set 把 originalName 覆写成了当时已带后缀的名字;clear 的恢复条件 remote.name === local.managedName 命中的是这个被污染的值。对照组:正常 set/clear 循环 3 次名字稳定在 Project,不累积。建议 set 前先剥掉自己的后缀,或在 originalName 已存在时不覆写。
N2|负责人生效后,每条未 @ 的群消息都多一次飞书 API 往返。 isChatManager 在 checkGroupMessageAccess 里对每条消息实时读群信息、且刻意不做缓存("标记消失、换人或读取失败时不使用旧缓存",这个取舍我理解)。实测:负责人启用时 10 条未 @ 消息 = 10 次远端读;未启用时因本地 claim 先短路只有 1 次。所以成本只落在真正启用负责人的群上,量级可接受,但活跃大群里这是每消息一次串行网络调用(larkGet 未设 timeoutMs),飞书抖动时会拖慢该群所有未 @ 消息的判定。可考虑加一个很短的 TTL(1–3s)或与 getGroupStats 那样的缓存对齐,同时保留失败即 fail-closed 的语义。
已复核通过的部分
tsc --noEmitrc=0、bun run buildrc=0(含 dashboard bundle 与制品检查)。- 新增 29 个用例全绿;我做了 7 枪反向变异(去掉本地 claim 要求 / 去掉显式管理员门 / 去掉
mentionsAnotherMember让路 / 跳过写后读回 / 去掉话题群拒绝 / 去掉描述长度门 / 去掉重复 marker 拒绝)—— 7/7 全部转红,测试是真承重的。 - 权限边界复核:open 模式(无 allowlist)下真人陌生人发
/manager set被正确拒绝(回"只有目标机器人的 owner/allowedUsers 可以设置或取消负责人"),没有回退到会话 owner。 - 四种
regularGroupReplyMode(chat / new-topic / chat-topic / shared)下负责人免 @ 寻址均按各自路由语义生效。 - 全量单测:本机负载很高(并发 vitest 进程数百),首轮 PR 侧 93 failed / master 侧 54 failed;把 PR 独有的失败文件单独重跑后从 21 个文件收敛到 3 个,失败原因是 30s 超时、空 TUI、以及我本机
bun 1.4.0≠要求的1.4.2(这条在 master 侧同样出现 19 次)。除上面那 2 条command-handler.test.ts之外,没有发现本 PR 引入的回归 —— 那 2 条在低负载下单文件复跑仍稳定复现,且 master 单文件跑 320/320 全绿。
⚠️ 关于 CI
这个 PR 的 CI 从未真正跑过:run 34194924981 的 conclusion=action_required(fork PR 等维护者放行),status=completed 看起来像跑完了但其实一行没跑,gh pr checks 和 commits/<sha>/check-runs 都返回空。所以上面那 2 条红是尚未被 CI 发现的 —— 放行后必需的 test check(3 shard AND)会红。
以上是自动评审的初步意见,可能有误判,最终以维护者审阅为准。辛苦了 🙏
补充:给上面那条阻断项一个更干净的修法(已实测)复审后我们把修法又推进了一版。之前我推荐的 (B)(保留 (B′) 做两件事① 把 你加这条的真实诉求是「别让用户用 export const INGRESS_RESERVED_COMMANDS = new Set(['/manager']);
// normalizePassthroughCommand 里加一行:
if (INGRESS_RESERVED_COMMANDS.has(normalized)) return null;这样一次解决四件事:
幽灵会话是什么:
② 仍然把
// src/i18n/zh.ts(en.ts 同理,i18n 必须双语成对)
'help.manager': '@机器人 /manager [status|set|clear] - 普通群负责人:set 设为默认接待者(顶层免 @ 应答)|clear 取消|status 查看',
// src/core/command-handler.ts 的 /help 数组,紧跟 t('help.reply_mode') 之后
t('help.manager', undefined, loc),并建议把 (B′) 实测结果改动共 5 文件 16 行( 并逐项确认: 另外建议本 PR 内一并修 N1(群名后缀累积)上一条评论里的 N1 有个很小的修法: 一处更正上一条评论我说「同族 ingress 命令都在 以上仍是自动评审的初步意见,最终以维护者审阅为准。(B′) 的改动我只在本地验证,未推送到你的分支。辛苦 🙏 |
关于必要性:能否说明为什么不复用已有的
|
ambient(已有) |
负责人(本 PR) | |
|---|---|---|
| 免 @ 应答顶层群消息 | ✅ | ✅ |
| @ 了别人时让路 | ✅ | ✅ |
@all 不算让路 |
✅ | ✅ |
受 canTalk 约束 |
✅ | ✅ |
| 按群配置 | ✅ chatMentionModes[chatId] |
✅ 群描述标记 |
| 全群互斥(只能有一个) | ❌ 各 bot 各存自己的配置,多个 bot 可同时开 | ✅ |
| 交接协议(旧的先 clear) | ❌ | ✅ |
| 群名可见谁在值班 | ❌ | ✅ |
也就是说本 PR 相对 ambient 真正新增的是后三项。我实测确认了互斥确实生效(三个 bot 依次 set,第二三个得到 manager_already_set,同时生效的负责人数 = 1;clear 后交接成功、群名从 G · A 变为 G · B)——这个能力 ambient 确实提供不了,因为 ambient 存在各 bot 自己的配置里,天然无法表达「全群唯一」。所以你选「写进飞书群描述」作为跨机共享点,这个方向本身是合理的。
希望你补充的是:能否在 PR 描述里说明,为什么选择新开一套机制(新命令 /manager + 新持久化层 chat-managers/ + 群描述标记),而不是在现有的 mention-mode 体系上扩展(例如给 ambient 加一个「独占」变体,共享点仍可以用群描述)?维护者关心的是为这三项收益引入一套新的跨机状态是否划算。
如果有 ambient 无法承载的具体原因(比如交接语义与 mention-mode 的四档模型冲突、或群名展示必须跟状态强绑定),写清楚会很有帮助;这决定的是走「合本 PR」还是「扩展 ambient」,而不是代码质量问题。
顺带重申一下前两条评论的技术结论,方便你一次看全:
- 🔴 合并前需修:
/manager加进DAEMON_COMMANDS打红了 2 个主干既有单测(command-handler.test.ts的 size 断言 +/help遍历契约)。修法 (B′) 我已实测(改用独立的INGRESS_RESERVED_COMMANDS挡透传 + 照/grant惯例把/manager加进/help):5 文件 16 行、495/495 绿、tscrc=0、bun run buildrc=0,且不需要改任何断言数字,同时顺带消掉幽灵会话与canTalkDaemonCommands静默无效配置。详见上一条评论。 - 🟠 建议本 PR 内修 N1:群名后缀会累积且原名不可恢复(人在飞书 UI 删掉描述里的 marker 行后再
set就二次追加)。修法:set时若本地 claim 已存在且remote.name === local.managedName,用local.originalName作拼接基准,且不要用已带后缀的名字覆写originalName。 ⚠️ CI 从未真正跑过:run34194924981的conclusion=action_required(fork PR 等维护者放行),所以上面那 2 条红尚未被 CI 发现。
以上仍是自动评审的初步意见,最终以维护者审阅为准。感谢你的耐心 🙏
变更内容
新增普通群负责人命令
@目标机器人 /manager set|clear|status:为什么
普通多机器人群需要一个默认接待者,但逐个配置 ambient 无法表达负责人交接。把免 @ 寻址、管理员操作和跨部署标记分开,可以保留现有授权规则,避免因为换负责人而扩大机器人通信权限。
这是轻量的群负责人机制,与 #1307 的项目状态、进度卡和 dispatch/report 编排不是同一个功能。本 PR 不引入项目卡、任务派发、共享模型上下文或非负责人行内回复规则。
影响范围与边界
验证
在独立 macOS checkout 验证,未连接或重启运行中的机器人:
群名效果示例:
Project→Project · Manager→ 取消后恢复Project;期间用户手动重命名则保留用户的新名称。