本周我给 Tently 修了一个 bug。Tently 是我们做的 PR 审查工具。修完之后测试全绿:数据库包 77 个、API 251 个、worker 190 个,一个没挂。
然后 Tently 审了我这个 PR,挑出两个测试没覆盖到的问题。一个出在我没考虑到的调用方身上,另一个出在 diff 压根没碰过的文件里。我觉得这类漏网正是 agent 写的 PR 更容易犯的,所以把过程记下来。
先说 bug 本身
Tently 存着团队做过的各种决定,比如"核心包和 reviewer 包里不许有 I/O""LLM 调用一律走 router 包"。每条决定都带一个作用域,是一组路径 glob,形如 packages/reviewer/**。
录 demo 的时候,我让 Cursor 直接从 reviewer 包里调 GitHub API。Cursor 先调了 Tently 的搜索,把即将编辑的目录 packages/reviewer 传了进去。搜索结果返回了一堆流程规则,唯独漏掉了最要紧的那条——那条标着 BLOCKING。
原因就一行。像 packages/reviewer/** 这样的 glob 编译成的正则,要求斜杠后面必须还有东西。光秃秃的目录名永远匹配不上,于是所有作用域挂在这个目录上的决定,全都从搜索结果里消失了。
修复和测试给出的假象
修法看着简单:路径里没有文件名,就当它是目录,凡是 glob 能伸进这个目录的决定都算匹配。我判断"没有文件名"的规则是"最后一段没有点",于是 packages/reviewer 算目录,src/app.ts 算文件。
我给目录、父级、兄弟、排除项都写了测试,全过。PR 描述里我写了一句:文件路径的行为不变。
第一个发现:Makefile 也没有点
Tently 的审查回来,给了一条中等严重度的发现,大意是:Makefile、Dockerfile、LICENSE 这类没有扩展名的文件,现在被当成了目录。它们的排除路径被拿去和 path/ 比对,而不是和路径本身比对,于是按精确文件名写的排除项不再能拦下它们。这会影响 PR 审查器,而它传的是真实文件路径。
说得对,而且比听起来严重。团队用排除路径来记录已批准的例外:"这条规则管 apps/api/**,但 apps/api/Makefile 除外"。我改完之后,审查器会把 apps/api/Makefile 读成目录,去检查排除项有没有覆盖 apps/api/Makefile/。没有,于是例外失效了。
我改的那个函数,编码 agent 调的搜索和 PR 审查器跑的搜索是共用的。我脑子里只想着前一个。
测试之所以全过,是因为没有一个测试用到"无扩展名文件 + 排除项"这个组合。
修法:一个没有点的路径,既可能是目录也可能是文件,两种读法只要有一种说匹配就算匹配,排除项同理。只有结尾带斜杠才强制按目录处理。
第二个发现:diff 没碰过的文件
第二次审查发现的东西完全在 diff 之外。仪表盘里有一份同样的匹配器拷贝,因为 web 应用没法 import 数据库包。这份拷贝的文件头写着它是"逐字移植"的,必须保持同步。
我的 PR 改了原件,没改拷贝。Tently 把这个缺口标了出来,还指出拷贝在我这个 PR 之前就已经漂了:它把排除项套在整个 PR 的所有文件上,而不是逐个文件套。等我去同步改动时,又发现第二处漂移:它处理 **/ 的方式会让 a/**/b 匹配上 a/cab。也就是说,PR 页面列出的决定,可能和审查器实际执行的不是同一批,而且这个状态已经持续了一段时间。
我的 diff 没有一行碰过那个文件。只读 diff 的审查看不见它。Tently 能找到,是因为那个文件里有一条注释写着一条规则——"保持两份同步"——而我的改动违反了这条规则。
我的收获
两个 bug 都不离奇。都是那种细心的审查者只要把系统其余部分装在脑子里就能抓住的问题:还有谁在调这个函数,还有谁承诺过要和它保持同步。这恰恰是作者最先忘掉的部分,不管作者是人还是 agent,因为 diff 看起来已经完工了。
这个修复 PR 最后是三个 commit 而不是一个,仪表盘那份拷贝现在和原件逐字节一致,测试也一并同步了。
特别声明:以上内容(如有图片或视频亦包括在内)为自媒体平台“网易号”用户上传并发布,本平台仅提供信息存储服务。
Notice: The content above (including the pictures and videos if any) is uploaded and posted by a user of NetEase Hao, which is a social media platform and only provides information storage services.