Skip to content

fix(claude-hook): 识别开发版平台二进制 - #1277

Merged
deepcoldy merged 2 commits into
deepcoldy:masterfrom
LPX-E5BD8:fix/dev-binary-hook-dedup
Sep 7, 2026
Merged

fix(claude-hook): 识别开发版平台二进制#1277
deepcoldy merged 2 commits into
deepcoldy:masterfrom
LPX-E5BD8:fix/dev-binary-hook-dedup

Conversation

@LPX-E5BD8

@LPX-E5BD8 LPX-E5BD8 commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

问题

Hook 去重只识别 cli.jsbotmuxbotmux.exe。本地 bun run use:here --binary 使用 dist-bin/botmux-<platform>-<arch>[-musl],因此路径切换或重复安装时,SessionStart/UserPromptSubmit hook 会持续累积。

Fixes #1210

修复

  • 将 BotMux 可执行文件 basename 判据扩展为受限的平台产物枚举:Linux、Darwin × x64/arm64(可选 -musl),以及 Windows × x64/arm64(可选 .exe)。
  • 不使用宽泛 botmux-* 匹配,继续拒绝 botmux-helperbotmux-wrapper、缺平台/架构和额外后缀等第三方名字。
  • 回归测试覆盖 Ask、SessionStart、UserPromptSubmit 三类 hook 在不同 dist-bin 路径间重复安装后仍各保留一条。

影响面

  • 只影响 Claude 家族 hook 的幂等识别与清理。
  • 生产 npm/install.sh 的 botmux 名称和 Node cli.js 路径行为不变。
  • 不扩大到任意同前缀程序,避免误删第三方 hook。

验证

  • npx vitest run --project unit test/hook-installer.test.ts:36 passed。
  • bun run vitest run --project unit test/hook-installer.test.ts:36 passed。
  • bun run build:通过。

Copilot AI lite review requested due to automatic review settings September 6, 2026 10:37
@LPX-E5BD8
LPX-E5BD8 requested a review from deepcoldy as a code owner September 6, 2026 10:37

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

isBotmuxHookCommand 的新正则会将非 Windows 平台的 *.exe 也识别为 botmux hook,存在误判并可能误删第三方同前缀 hook 的风险。

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

该 PR 修复 Claude 家族 hook 去重逻辑在开发态 dist-bin/botmux-<plat>-<arch>[-musl] 二进制形态下无法识别自身可执行文件的问题,避免在路径切换/重复安装时 SessionStart / UserPromptSubmit 等 hook 持续累积,符合 botmux 的 hook 幂等安装目标。

Changes:

  • 扩展 Claude hook 调用目标 basename 的白名单识别逻辑,覆盖 dist-bin/botmux-<plat>-<arch>[-musl] 形态。
  • 新增回归测试:验证 dev dist-bin 路径互切时三类 hook 仍保持幂等;并验证不误伤 botmux-helper 等第三方同前缀程序。
File summaries
File Description
src/adapters/hook-installer.ts 扩展 botmux hook 调用目标的 basename 识别规则以覆盖 dev dist-bin 产物
test/hook-installer.test.ts 增加 dev dist-bin 幂等与第三方同前缀不误判的回归用例
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/adapters/hook-installer.ts Outdated
Comment on lines +142 to +143
return basename === 'cli.js'
|| /^botmux(?:-(?:linux|darwin|windows)-(?:x64|arm64)(?:-musl)?)?(?:\.exe)?$/.test(basename);
@LPX-E5BD8
LPX-E5BD8 force-pushed the fix/dev-binary-hook-dedup branch from 11903cd to 4c827ab Compare September 6, 2026 10:49
@deepcoldy

Copy link
Copy Markdown
Owner

你好!这是 Botmux 的自动评审流程。

本 PR 的评审群已创建:https://applink.feishu.cn/client/chat/open?openChatId=oc_996227482979275a8188d8b2683024dc

但你暂时未被拉入群中——你的 GitHub 账号(LPX-E5BD8)不在自动拉群名单里。请把你的 GitHub 账号和飞书信息补进名单文档:
https://bytedance.larkoffice.com/wiki/WJ1nwWbtxi89erkNGNbcgkt9nUe

补好后,后续复审会自动把你拉进群。感谢贡献!

@deepcoldy

Copy link
Copy Markdown
Owner

感谢这个 PR,方向和实现都对,生产代码部分我这边没找到阻断问题。先说结论:改动逻辑正确,复现与修复都验证通过;两条正则甚至比 issue #1210 里建议的写法更严谨。 下面两条都属于「测试有效性」的建议,不影响生产行为。

先肯定一点:你的正则比 issue 的建议更好,别改回去

issue #1210 建议的是单条 ^botmux(-(?:linux|darwin|windows)-(?:x64|arm64)(?:-musl)?)?(\.exe)?$。我把两版对 20 个 basename 逐个跑了一遍,你拆成「linux/darwin 带可选 -musl」+「windows 带可选 .exe」两条之后,多拒掉了三个自相矛盾的组合:

basename 你的实现 issue 建议
botmux-linux-x64.exe 拒绝 ✅ 误收 ❌
botmux-darwin-arm64.exe 拒绝 ✅ 误收 ❌
botmux-windows-x64-musl 拒绝 ✅ 误收 ❌

11 个合法形态两版都命中。这个差异是你的实现更贴合 build-bun-binary.mjs 的真实产物规则(musl 只在 linux 出、.exe 只在 windows 出),建议保留。

修复本身有效(我复现了)

用带引号的真实形态 "<repo>/dist-bin/botmux-linux-x64" session-ready 连装三次:

  • 基线(最新 master):PreToolUse / SessionStart / UserPromptSubmit = 1 / 3 / 3,与 issue 里的实测口径逐字一致
  • 打上本 PR:1 / 1 / 1

F1(建议合入前修,非阻断):7 条负向用例目前不承重

不把第三方同前缀程序 %s 当成 botmux hook 这 7 条,实测对任何变异都免疫:

  • isBotmuxHookCommand 整体改成 return true → 7 条仍全绿
  • 换成 issue 明文禁止的宽匹配 ^botmux(-[\w.-]+)?(\.exe)?$36/36 全绿

原因是断言走的 hasInstalledSessionReadyHook 用的是精确字符串相等entry.command === hookInstall.sessionStartCommand),函数体内根本没有调用 isBotmuxHookCommand;而用例写进配置的是 /usr/local/bin/botmux-helper session-ready、查询传的是 /repo/dist-bin/botmux-linux-x64 session-ready —— 两个不同字符串,精确比对必然 false,跟白名单宽窄无关。

这条正好是 issue 验收建议里点名要锁的性质("同时断言 botmux-helper 这类第三方 hook 不被删除"),代码确实做对了,只是测试没锁住。

可行的改法是改成从真正会调用该谓词的入口(removeBotmuxReadyHookGroupsisBotmuxReadyHookGroup)进,断言第三方命令仍然存在

installHook('claude-code', {
  configPath, format: 'claude-settings',
  sessionStartCommand: '/repo/dist-bin/botmux-linux-x64 session-ready',
}, '/repo/dist-bin/botmux-linux-x64 hook claude-code');

const cmds = JSON.parse(readFileSync(configPath, 'utf-8'))
  .hooks.SessionStart.flatMap((g: any) => g.hooks.map((e: any) => e.command));
expect(cmds).toContain(thirdPartyCommand);   // 第三方 hook 必须存活

我本地验过这个写法:当前实现下 7/7 绿,换成宽匹配后 7/7 红 —— 是承重的。

F2(非阻断):-musl / darwin / arm64 三个分支目前没被覆盖

把这三个分支分别删掉(如 (?:-musl)? 去掉、枚举里去掉 darwin、去掉 arm64),都还是 36/36 绿;只有 windows 分支真被钉住了。

原因是「装 A 再装 B,断言剩 1 条」这种去重用例只对先装的那个 basename 施加压力,后装的那个只要能写进去就行、不需要被识别。(?:-musl)? 更彻底,它没出现在任何用例里。

建议补一组逐形态的正向断言(it.eachbotmux-linux-x64-musl / botmux-linux-arm64-musl / botmux-darwin-x64 / botmux-darwin-arm64 各自被识别)。-musl 在开发路径是可达的:use:here --binary 接受显式路径,build:bun --all 也会产出这些产物,所以覆盖它是对的,只是需要测试钉住。

我这边跑过的验证

在 rebase 到最新 master(80474f414,零冲突,两文件 blob 哈希逐字未变)之后:

  • tsc --noEmit:0 error
  • bun run build:通过
  • npx vitest run --project unit test/hook-installer.test.ts:36 passed
  • bun test test/hook-installer.test.ts(真 Bun 运行时):36 pass / 0 fail
  • hook 相邻 6 个文件(hook-installer / binary-self-update / autostart-standalone-path / claim-botmux-bin-binary / botmux-wrapper / wrapper-standalone-guard):143 passed

另外提醒:这个 PR head 上的 CI 目前还是 action_required(fork PR 待批准),check-runs 数为 0,也就是尚未真正跑过 CI,需要维护者批准后才有结论。


以上是自动评审的初步意见,F1 / F2 都只涉及测试、不影响生产行为,是否需要在合入前处理请以维护者的审阅为准。

@deepcoldy

Copy link
Copy Markdown
Owner

补充一条对上面 F1 的更正,以及两处我自己的数字纠错。复审又跑了一轮,结论从「建议合入前修 F1」放宽为「F1 取决于 #1276 的合并顺序,可能无需你动手」。

更正(重要):F1 在 #1276 之后会自动闭环

你的 #1276hasInstalledSessionReadyHook 从精确字符串相等改成了结构化匹配(groups.some(g => isBotmuxReadyHookGroup(g)))—— 那正是让本 PR 这 7 条负向用例哑弹的同一个 helper。我把两个 PR 都 rebase 到最新 master 后真合了一遍:

也就是说 #1276 一旦先合,本 PR 这 7 条用例就自动变成承重的,你不需要为 F1 改任何测试。上面 F1 那段给的替代写法只在「#1277 先合、#1276 还没合」的情况下才有必要。

F2 不会自愈:合并态下把 -musl / darwin / arm64 分别删掉仍各自 38/38 全绿。

两个 PR 的冲突情况(供你参考)

两者都改 src/adapters/hook-installer.ts + test/hook-installer.test.ts

冲突不难解,但有个坑值得提前说:冲突切口落在 it() 内部,所以不能把两段直接拼起来(那样会把 #1276 那条用例截成半截,报 [PARSE_ERROR] Unexpected token,vitest 显示 Tests no tests)。正解是取一侧文件整份、再把另一侧新增的完整 it 块整块插到块边界。我本地这样解完 tsc --noEmit 0 error、38/38 绿。

F2 的一行改法(比我上面写的更省)

不用新增 it.each,只要把第一个幂等用例里先装的那个串换掉即可 —— 因为去重语义只对先装的那侧施压,所以把最全的形态放在先装位:

const first = '/repo/dist-bin/botmux-darwin-arm64-musl';   // 原 botmux-linux-x64
const second = '/repo/dist-bin/botmux-linux-x64';          // 原 botmux-darwin-arm64

实测这一行之后,darwin / linux / arm64 / x64 / -musl 五个分支逐个删掉全部转红。(顺带一提,只换成 linux-arm64-musl 是不够的 —— 那样 darwin 仍然全绿。)

两处我自己的数字纠错

  1. 上面我说「5 个调用点」,实际是 4 个(164/187/244/266 行,ask/traex/ready/prompt 四条路径)—— 我把 134 行的函数定义误数成了调用点。
  2. 「PR 正则比 issue 建议多拒 3 个组合」这个数字偏低:对 plat×arch×musl?×exe? 全 24 组合穷举,你的接受集是 issue 建议的严格子集(你独占 0 个),多拒的是 12 个,全是自相矛盾组合(*-linux-x64.exe*-windows-*-musl 等)。方向和结论不变,你的写法更严更对。

以上依然是自动评审意见,合并顺序与是否要动 F2 请以维护者的判断为准。

@deepcoldy deepcoldy left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

复审通过,三条评审意见(N1 / F1 / F2)都已在 c2a413103 落实,感谢跟进。

新 head 我从头重验了一遍(rebase 到最新 master 02dea1044,零冲突):

  • tsc --noEmit:0 error
  • npx vitest run --project unit test/hook-installer.test.ts48 passed
  • bun test test/hook-installer.test.ts(真 Bun 运行时):48 pass / 0 fail
  • 真编 dist-bin/botmux-linux-x64 跑端到端(用 hook-command.ts 自己渲染的形态连装三次):1 / 1 / 1,基线上同样测法是 1 / 3 / 3

三条意见的落实我都做了反变异确认,不是只看绿:

验证
F1(负向用例承重) 过宽正则变异 → 11 条转红(改前是 0 条);return true → 同样 11 条红
F2(枚举分支覆盖) 7 个分支逐个删(linux-x64 / linux-arm64 / -musl / darwin-x64 / darwin-arm64 / windows-arm64 / .exe 可选性)全部转红
N1(darwin-musl) 穷举 plat×arch×musl?×exe? 全 24 组合:6 个真实 RELEASE_TARGETS + 4 个 windows 本地产物全接受,接受但非真实产物 = 0

另外补充一条前几轮没覆盖的:isBotmuxHookCommand 还喂着一条破坏性路径(isBotmuxTraexAskHookEntrycleanupTraexAskHooks,由 worker-pool.ts:3874 对用户的 ~/.trae/hooks.json 真删条目)。这条路径上也验了:9 个同前缀第三方命令全部存活、dev 二进制的 traex hook 正常清理,过宽变异下 9 条转红。

顺带说明:darwin-*-musl 的收紧让这里和 src/core/binary-install-shape.ts:102/180 的正则出现了差异(那两处仍是 (?:linux|darwin)-…(?:-musl)?)。那两处是既有代码、不在本 PR 范围内,也不影响本 PR 正确性 —— 如果以后要统一,适合单独提一个 PR。

LGTM。

LPX-E5BD8 and others added 2 commits September 7, 2026 16:28
扩展 BotMux hook 命令的 basename 白名单,精确覆盖 dist-bin 生成的平台二进制名,并补充三类 hook 的幂等回归以及第三方同前缀程序反例。

Refs: deepcoldy#1210

Co-authored-by: TRAE CLI <noreply@bytedance.com>
@LPX-E5BD8
LPX-E5BD8 force-pushed the fix/dev-binary-hook-dedup branch from c2a4131 to e9a04db Compare September 7, 2026 08:31
@LPX-E5BD8

Copy link
Copy Markdown
Contributor Author

Rebased onto current master so this sits on top of #1276. Same change: recognize the real dist-bin platform binaries, keep the third-party prefix cases out.

@deepcoldy
deepcoldy merged commit 7326f93 into deepcoldy:master Sep 7, 2026
5 checks passed
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown

🚀 Released in v3.19.3

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.

fix(claude-hook): hook 去重 basename 白名单未覆盖 dev 的 dist-bin 二进制形态

3 participants