ARTICLE DETAIL

资讯详情

深耕网站建设、视觉设计与SEO优化的一线实战洞察。

Hermes:基于GitHub生态的自动化代码评审工具构建实践

Hermes:基于GitHub生态的自动化代码评审工具构建实践 1. 代码评审的日常之痛Hermes要解决的真实问题先说说我为什么折腾这个项目。我所在的团队大概有二十多个研发分布在三个不同的业务小组里GitHub 是唯一的代码托管平台。PRPull Request是我们每天都要打交道的东西按理说流程已经跑得很顺了但真正让所有人头疼的恰恰就是这个“顺”字——PR 太多了多到评审根本看不过来。刚开始大家还守着“每个 PR 必须至少两个人 review”的规矩后来发现根本执行不下去。业务迭代快的时候一天能开二三十个 PR一个 PR 少则两三百行多则上千行。每个人手头都有开发任务抽出整块时间去看别人的代码本来就不现实更别提还要理解对方的业务上下文、检查潜在的边界条件、验证改动是否破坏了其他模块。于是 review 逐渐演变成了一种形式看一眼标题扫一遍 diff没问题就 approve。等到合入主干之后出了线上问题再回头翻 PR 记录才发现当时根本没人注意到那个隐患。1.1 评审人效瓶颈与“假性通过”我印象最深的一次事故是某个服务在发布之后内存持续飙升最后 OOM 重启。查了半天根因定位到最后一次 PR 的改动——有人在配置中心加了一个缓存开关默认值设置成了true但那个缓存组件在极端并发场景下会持有大量对象引用旧版本代码里一直是关闭的。这个 PR 代码量不大diff 只有四十多行review 的人也确实看了但问题在于评审者对这个缓存组件的内部实现不够熟悉压根没想到一个开关能引发内存问题。这就是人工评审的天然短板人对不熟悉的代码区域敏感度会急剧下降。四十行代码如果里面涉及的是评审者每天都在写的业务逻辑他可能一眼就看出问题但如果是底层组件、配置项、依赖升级这类“看起来人畜无害”的变更多数人会选择直接放行。不是说这些人不负责而是人的注意力带宽本来就有限。后来我统计过团队过去三个月的 PR 数据发现平均每个 PR 从发起到合入要经过 4.2 次提交更新而其中大约 30% 的 PR 在第一次 review 时就会被提出修改意见这些修改意见里有将近一半是低级的格式问题、明显的空指针风险、敏感信息泄露这类“机器本可以自动发现”的问题。换句话说评审者的大量精力被浪费在了本该由工具自动拦截的事情上而真正需要人工深度思考的设计问题、逻辑问题反而因为前面那些琐碎问题消耗了耐心最后草草收尾。1.2 自动化审查的定位不是替代人工而是做人工的过滤器所以 Hermes 这个项目的定位从一开始就很明确它不是要替代人工评审而是要在人工评审之前先把那些机械性、规律性、可脚本化的问题全部筛掉把评审者的注意力从“找茬”转移到“思考”上。打个比方人工评审就像一个语文老师批改作文他既要看错别字、病句又要看文章立意、结构、逻辑。但如果每篇作文都有一堆错别字老师批改的时候就会很烦躁很难静下心来看立意。Hermes 想做的事情就是像 Grammarly 那样先把错别字和病句标出来让老师只需要专注于评判文章本身。有了这个定位后面所有设计决策都围绕一个原则展开自动化审查结果必须足够可信。你拦下一个错误团队会觉得这工具不错但如果你拦下的十个问题里有五个是误报团队很快就会丧失对它的信任最终沦为又一个被关掉的机器人提醒。2. Hermes的整体设计从PR事件到审查报告的完整链路再聊聊整体设计。Hermes 这个名字取自希腊神话里的信使神寓意“传递信息”。它在 GitHub 生态里的角色就是一个监听 PR 事件、拉取代码上下文、执行检查逻辑、再把结果回传给 PR 评论区的自动化助手。2.1 核心工作流整个系统的工作流可以拆成五个环节事件捕获监听 GitHub 的pull_request事件包括 opened、synchronize、reopened以及pull_request_review_comment事件用于处理用户对机器人评论的反馈。任务分发把事件消息投递到任务队列由 worker 异步处理。这一步很关键因为 GitHub Webhook 的响应超时只有 10 秒你必须在 10 秒内返回一个 200 响应否则 GitHub 会认为投递失败并重试。重试本身不可怕可怕的是重试导致同一事件被重复处理最后生成一堆重复评论。上下文拉取根据 PR 的 head 分支和 base 分支拉取完整的 diff 数据同时获取相关文件的最近提交历史、涉及模块的依赖关系等元信息。检查执行把 diff 数据喂给一组检查器Checker并发执行。每个 Checker 负责一类特定的问题比如代码风格、空指针风险、敏感信息扫描、配置项变更检查等。结果回写汇总所有 Checker 的输出按严重级别分级写入 PR 评论区。同时更新 PR 上的检查状态status check这样 GitHub 的 branch protection 可以基于这个状态来决定是否允许合入。2.2 架构分层与模块职责我最终实现的架构比较朴素没有上什么微服务之类的花架子就是三个模块加一个队列。第一层是接入层Webhook Gateway它是一个很小的 Node.js 服务只做两件事验签和投递。验证 GitHub Webhook 的X-Hub-Signature-256签名确保请求确实来自 GitHub然后立刻把事件原样塞进 Redis 队列返回。这里不要做任何耗时操作包括去查数据库、拉取 diff 之类的全部丢给后面的 worker。第二层是执行层Worker这是核心逻辑所在。Worker 消费 Redis 队列里的事件执行完整的分析流程。它可以横向扩展PR 多的时候开多个 worker 实例因为每个任务的上下文是完全隔离的天然适合并行处理。Worker 本身是无状态的任何时刻宕机都不会丢数据——任务还在队列里重启之后继续消费就行。第三层是存储层我用 PostgreSQL 存检查结果的历史记录方便后续统计分析。比如哪些规则命中率最高、哪个目录的代码问题最多、团队的问题趋势是在变好还是变差这些数据对技术管理者来说是很有价值的。队列我选的 Redis BullMQ原因很简单团队里 Redis 已经是标配BullMQ 的 API 比 RabbitMQ 的 Node.js 客户端好用得多而且它对延迟任务、定时任务的支持很完善。后续如果要加“PR 提交后自动执行、五分钟后再跑一轮深度分析”这类需求BullMQ 的 delayed job 直接就能实现。2.3 为什么不直接买现成工具可能有人会说市面上已经有 CodeRabbit、Sourcery、GitHub Copilot Code Review 这类产品了为什么还要自己造轮子我的回答是工具能解决通用问题但解决不了边界问题。现成的代码审查工具擅长检测常规的代码质量问题但它们不懂你团队内部的规范和业务约束。举个例子我们团队有个约定所有对外接口的参数校验必须放在 Service 层不能依赖 Controller 层的注解——这个规则任何商业工具都不会知道但它们恰恰是团队内最容易犯的错误之一。Hermes 的可扩展性设计就是为了解决这个问题它内置一套规则引擎团队可以自己编写规则把“我们团队特有的红线”自动化。这才是我做这个项目最大的价值——不是替代 CodeRabbit而是用它兜住那些商业工具看不到的角落。3. 核心实现拆解静态分析、动态验证与报告生成这一趴是本文的重头戏。Hermes 的检查逻辑总体上分为三大块静态分析、动态验证和报告生成。3.1 静态分析不止是 lint刚开始做的时候我以为静态分析就是调用一下 ESLint 和 Prettier 之类现成的检查器把它们跑出来的结果格式化一下写进 PR 评论就完事了。后来发现自己想得太简单了。ESLint 确实能抓出代码格式、未使用变量、明显的空指针风险但它的能力边界停留在“单个文件的语法和局部上下文”层面。真正有价值的检查是那些跨文件的逻辑一致性。我举一个实际的例子。我们的项目里大量使用了配置中心配置项的命名规范是module.feature.enabled。但配置项的读取散落在各个业务代码里有的写的是getConfig(module.feature.enabled)有的写的是getConfig(module.feature.enable)少一个d。ESLint 完全发现不了这种问题因为它不会去分析配置项的注册列表和消费列表之间的对应关系。Hermes 怎么处理这类问题我用了一个简单但有效的方案在 Worker 里增加一个配置消费索引Config Usage Index。当一个 PR 改变了某个配置项的读取代码时Hermes 会去全局扫描整个仓库中所有对该配置项的引用检查引用是否一致。如果 PR 把module.feature.enabled改成了module.feature.enable但其他文件里还有三处引用旧名称Hermes 就会在 PR 评论中明确指出来。实现方式也不复杂就是在 Worker 启动时构建一个代码索引利用简单的正则和 AST 解析混合方案把每个文件里配置项的名称、所在行号、上下文摘录提取出来存进内存里的一个 Map。PR 到达时根据 diff 中涉及的文件和行号快速定位哪些索引条目被影响再做交叉比对。整个过程用不到什么高深的算法但对团队的实际帮助非常大。3.2 动态验证跑通一次最小的“冒烟链路”静态分析只能看到代码长什么样看不到代码跑起来是什么行为。所以 Hermes 还内置了一个动态验证的步骤——在受控环境里跑一次最小化的“冒烟验证”。这里说的冒烟验证不是指跑完整的测试套件那应该由 CI 流水线负责而是针对 PR 改动涉及的特定模块做一次轻量级的启动检查。比如 PR 改了数据库访问层的代码Hermes 会尝试在这个服务的测试配置下启动一个最小实例连上测试数据库如果没有测试库就使用 SQLite 的兼容模式检查是否有明显的启动报错、Bean 注入失败、SQL 语法错误等问题。这一步非常实用。因为很多问题在 code review 阶段完全看不出来只有在启动容器的时候才会暴露。我见过太多 PR 合并后 CI 跑挂了原因就是某个人改了一个方法签名但调用方没有被静态检查覆盖到。动态验证的设计上有几个注意事项必须沙箱化执行环境要和生产环境隔离不能访问真实的数据库、缓存和外部服务。我的做法是使用 Docker 容器挂载 VOLUME 后编译镜像再启动环境变量全部走测试配置。超时控制每个任务的执行时间不能超过 5 分钟超时直接终止任务并标记为“超时未验证”不能在动态验证上耗费太多资源。失败信息必须可执行如果启动失败Hermes 要把完整的错误堆栈、日志片段、涉及的文件和行号都贴到 PR 评论里不能只丢一句“启动失败”。3.3 报告生成怎么让开发者愿意看检查完要出报告这个环节很多人不重视但我觉得报告质量直接决定了工具的成败。试想一下你辛苦写完了代码提了 PR机器人跑过来咔咔咔贴了二十条评论你是什么感觉大概率是烦躁和抵触。但如果机器人的评论总共只有三条每一条都一针见血你说“问题确实存在改一下就好了”你的感觉就完全不一样了。Hermes 的报告采用三级分级机制error严重会引发运行时异常、内存泄漏、安全问题或数据不一致的变更。这类问题一旦发现Hermes 会设置 status check 为失败阻断合入配合 branch protection。warning警告潜在风险但不是必然触发比如边界条件没有覆盖、异常处理过于宽泛、配置项命名不一致等。这类问题不阻断合入但会在评论区标注“建议修改”。info建议代码风格、可读性优化、模式改进等。这类信息默认合并到摘要里不会单独逐条评论避免噪音。报告的最后Hermes 还会生成一个变更摘要用自然语言描述这个 PR 涉及了哪些模块、影响面大概有多大、有哪些风险点。这个摘要对评审者非常有用尤其是当一个 PR 比较大、涉及的目录跨越多个业务域的时候评审者可以通过摘要快速建立整体认知再决定从哪里深入看起。摘要的生成本质上是一个模板填充程序把各 Checker 的输出按固定句式组装比如“本次变更涉及 4 个文件主要修改集中在user-service模块的认证逻辑配置项auth.token.expire的默认值发生了变更请注意该变更会影响所有依赖默认值的客户端。”信息密度高且不废话。4. 部署实录GitHub App 配置与 CI 集成做好了核心逻辑接下来是让它真正跑起来。这一步的坑比我想象的多主要集中在对 GitHub 生态规则的不熟悉上。如果你也想部署自己的自动化代码评审服务这部分建议仔细看。4.1 环境选型与准备我部署环境用的是团队已有的 Kubernetes 集群服务本身以 Docker 镜像方式运行依赖的外部组件只有 PostgreSQL 和 Redis。GitHub 侧则注册了一个GitHub App而不是用传统 OAuth App 或者个人 token。为什么选 GitHub App 而不是个人 tokenGitHub App 有独立的身份和权限体系不会依赖某个人的账号。个人 token 一旦持有者离职、改密码、撤销权限整个自动化服务就废了。GitHub App 可以设置精确的权限范围比如只读代码、读写 PR 评论、读写 check runs最小化权限暴露面。GitHub App 天然支持多仓库不需要每个仓库单独配置一个 token。4.2 GitHub App 的权限清单最容易踩坑这块是我踩坑最多的地方单独用一节来讲。GitHub App 的权限是按分类设置的很多名字长得差不多但作用范围完全不同。我最终使用的权限清单如下权限项权限级别用途说明Pull requestsRead write读取 PR 信息、在 PR 下发表评论ChecksRead write创建/更新 check run配合 branch protection 使用ContentsRead读取仓库代码、获取 diff 数据MetadataReadGitHub 强制要求提供仓库基础元数据Commit statusesRead write可选的用于更新 commit 状态这里有一个特别需要注意的坑如果你想用 GitHub App 的身份在 PR 里发评论权限需要的是 “Pull requests” 的 Read write而不是 “Issues” 的。有很多教程会把这两个混为一谈导致你按教程配好了权限却在调用创建评论接口时收到 403。另外GitHub App 的 Webhook 事件订阅也要单独勾选。我的做法是只订阅pull_request这一个事件因为我的核心触发条件只有这一个。如果勾选了pull_request_review、pull_request_review_comment甚至issue_comment你会收到大量无关事件Webhook 处理器里就必须写一堆判断逻辑来丢弃它们浪费时间也容易出错。4.3 本地调试技巧GitHub App 的 Webhook 回调地址必须是公网可访问的 HTTPS 地址这在本地开发阶段是个障碍。我的解决方案是使用ngrok把本地服务的端口暴露到公网然后在 GitHub App 设置里把 Webhook URL 临时改成 ngrok 提供的地址。调试时还有个实用技巧GitHub 的 Webhook 管理页面支持“Redeliver”功能可以重新发送最近一次事件。这意味着你不需要每次都去创建一个新的 PR 来触发流程只需要在页面上点击 Redeliver 就能重放上一次的请求极大提升了联调效率。4.4 接入 CI 流水线部署好服务之后还要在仓库的 Branch Protection 规则里把 Hermes 的 Check 加入“Required status checks”这样 GitHub 就会强制要求 PR 通过 Hermes 的检查才能被合入。具体做法是进入仓库的 Settings → Branches → Add rule在 “Require status checks to pass before merging” 里勾选 Hermes 对应的 check name。这样设置之后所有 PR 在合入之前都必须等待 Hermes 的检查完成且结果为成功。如果 Hermes 发现了严重级别的 error它会主动把自己的 check 状态标记为失败Branch Protection 就会自动阻断合入。这套流程跑通之后团队对 Hermes 的信任就建立起来了——它成为一个真正有约束力的质量关卡而不是一个只会发评论的聊天机器人。5. 真实运行效果与踩坑记录系统上线之后的第一个月我记录了大量第一手数据也踩了好几个之前没想到的坑。这里挑几个值得说的。5.1 一次真实的中等规模 PR 审查过程举一个很有代表性的例子。某个后端服务做一次配置重构PR 大概涉及 14 个文件、560 行改动。人工评审的话经验丰富的工程师大概需要 30 到 40 分钟才能完整过一遍经验少一些的可能要看一个小时。Hermes 的处理结果是这样的静态分析发现了 6 个 warning其中 3 个是配置项名称引用不一致2 个是边界条件没有做防空判断1 个是接口返回值没有包含错误信息。动态验证顺利通过服务实例正常启动。变更摘要明确指出这次重构会影响 3 个下游服务的配置解析逻辑建议重点测试支付回调接口。评审者拿到这份报告之后只需要把精力集中在摘要里提到的那几个重点区域以及作者对 6 个 warning 的回复。整个评审过程在 15 分钟内结束而且质量比之前明显更高——因为评审者的注意力被锚定在了真正需要思考的地方。5.2 踩坑记录二GitHub API 限流上线第一天就遇到了 GitHub API 的限流问题。GitHub 的 REST API 在没有认证的情况下每个 IP 每小时只能请求 60 次即使使用 GitHub App 认证也有每小时 5000 次的限制。听起来很多对吧但如果你频繁拉取 diff、获取文件内容、查询 commit 状态几千次配额不到一个小时就能用完。我当时的解决方案有三个开启 Conditional RequestsGitHub API 支持If-None-Match请求头文件没有变更时会返回 304不消耗流量。但这个特性要在 HTTP 客户端层面做封装不能每次请求都直接发。做本地缓存同一个仓库的 diff 数据在短时间内是稳定的我加了一个内存缓存缓存时间 5 分钟。同一个 PR 在同一分钟内如果被重复触发比如作者连续 push第二次直接走缓存。轮转 token如果服务承担多个仓库的审查可以把多个 GitHub App 实例的 token 放在一个池子里轮换使用。但这个方法比较 hack能不用就不用主要还是靠前两个方案节省配额。5.3 踩坑记录三diff 上下文的贫瘠与误报率这个问题很有代表性值得单独说一说。GitHub 提供的 PR diff 数据默认只包含每个 hunk 的上下文默认前后各 3 行。如果你的检查器需要知道某个变量是在哪里定义的这个变量在函数内部是否被重新赋值那 3 行上下文远远不够。Hermes 早期版本就因为这个吃了不少亏。比如一个检查器看到“某个函数内部有delete user.phone”这样的代码就基于这 3 行上下文给了一个 warning “删除用户手机号可能违反数据保留策略”。但实际上这个user对象在引入之前上游已经有权限校验而且phone字段在数据库中本来就是可空的这个delete操作只是个内存对象的属性清理影响完全可控。后来我修改了检查器的逻辑如果某一行代码的分析结论依赖的上下文信息不完整就标记为“无法判定”而不是强行给结论。这个改进把误报率降低了大概 40%代价是有一部分问题机器选择了沉默但我认为这是值得的——机器沉默最多就是漏掉一个问题后续人工评审还能补上机器乱说话团队要么被淹没在噪音里要么直接关掉它这才是最坏的结局。5.4 踩坑记录四评论消息堆积与 PR 噪音最初版本的 Hermes 会在每个 Commit 更新后重新执行检查然后把新生成的评论追加到 PR 评论区。结果一个 PR 如果被提交了 5 次评论可能累计 30 多条评论区完全变成了废话集合。后来我引入了评论折叠策略同一个 PR 的检查结果只保留一条“汇总评论”新的检查结果会以 edit 方式覆盖旧评论的内容而不是新建一条。同时如果检查结果比上一轮有改善比如 warning 数量从 6 个降到 2 个Hermes 会在汇总评论里用一句话说明“相比上一轮提交减少了 4 个问题”。这一个小小的改动对开发者的使用体验提升是巨大的。PR 评论区的核心价值应该是有状态的讨论而不是流水账式的机器提醒。6. 自定义规则引擎让 Hermes 长出团队的脑最后聊聊整个项目中最有价值、也最需要花精力把玩的部分——规则引擎。前文提到过现成的代码审查工具最大的问题是不懂团队内部规范Hermes 的自定义规则就是为了解决这个短板。6.1 规则包结构与语法设计Hermes 的规则引擎采用“规则包”的概念每个规则包是一个 JSON 配置加一段可执行的 JavaScript 逻辑以插件方式加载。一个最小化的规则包长这样{ name: team-security-rules, version: 1.0.0, rules: [ { id: NO_HARDCODED_SECRET, severity: error, description: 禁止在代码中硬编码 token/密码, match: { filePattern: **/*.{js,ts,py,java}, contentPattern: (api[_-]?key|password|secret)\\s*[:]\\s*[\][A-Za-z0-9/]{16,} } } ] }match里的filePattern和contentPattern是最常见的规则形式用 glob 和正则描述“什么样的文件里的什么样的内容会触发这条规则”。对于更复杂的场景可以提供一个handler.js文件导出任意 JavaScript 函数函数接收一个context参数包含文件内容、diff 数据、AST 解析结果等返回一个数组数组里的每个元素对应一条审查结果。6.2 编写自定义规则的实操案例我们团队有一个真实的规则直接作为案例讲更有说服力。背景我们有一个内部工具库company/logger统一封装了日志输出。团队规范要求所有业务代码里的日志输出都必须通过这个库禁止直接使用console.log。原因是统一日志库会附加 requestId、调用链信息直接使用console.log会导致线上日志无法关联调用链。对应规则脚本如下// 规则禁止在业务代码中使用 console.log module.exports function (context) { const results []; const lines context.diffLines; // diff 中新增的行 lines.forEach((line, index) { if (line.added /\bconsole\.(log|warn|info)\s*\(/.test(line.content)) { results.push({ severity: warning, message: 发现 console.log 调用请替换为 company/logger 的 logInfo/logWarn 方法, file: line.file, line: line.lineNumber, suggestion: import { logInfo } from company/logger;\nlogInfo(your message); }); } }); return results; };这个规则本身只有十几行代码但它实现了商业工具做不到的事情——把团队自己的工程文化沉淀成代码检查的一部分。新成员提 PR 时在最早期就会收到这样的提醒而不是等 code review 时被老员工口头教育。6.3 规则优先级与快速失败多规则并行执行时规则的输出结果可能存在冲突。比如一条规则说“这个变量命名不够清晰”另一条规则说“这个变量命名符合团队规范”到底听谁的Hermes 的规则引擎设计了一个简单明了的判定逻辑按规则的priority字段排序priority值小的先执行、先输出后执行的规则如果输出的结果和已有结果在同一文件同一行冲突后者的级别降一档。这个规则不复杂但能有效避免报告内部矛盾让开发者处理反馈时不需要在两堆建议之间纠结。另外对于操作非常耗时、或者资产场景在error级别的规则我建议开启快速失败。也就是说如果一条 error 级别的规则命中后续的 warning 和 info 级别检查可以先不执行直接汇报“严重问题优先处理”。不是为了偷懒而是给开发者最明确的下一步动作。结尾几点掏心窝的经验最后再说点我这个项目之外的心得。如果你也在考虑做自己的自动化代码评审工具我最想说的是别一上来就追求功能的完整先把你团队最痛的那两三个问题解决了让团队真正用起来再慢慢加东西。我这个项目最早只有一个空指针扫描规则后来是根据团队的反馈一步步迭代成现在这个样子的。用户的需求不是靠调研问卷问出来的是你在真实使用中观察和感受到的。另外工具做出来只是开始真正难的是让团队愿意用。我在这上面花的心思不比写代码少给每个新规则配一段易懂的说明文字被开发者驳回的问题记录并分析误报原因每个月在周会上展示一次 Hermes 拦截了哪些真实问题……这些看起来“很软”的事情决定了一个工具最终是活在 CI 线里还是死在 PR 评论区的噪音里。还有一个小技巧可以分享不要把所有检查结果都挤在一轮里输出。开发者在写代码的时候不希望被打断太多次。我现在的习惯是刚提交时只跑静态分析和配置检查等开发者把 review 意见处理完之后再跑动态验证和深度分析。分阶段的策略让整个提交、反馈、修改的循环顺畅很多。自动化代码评审这条路没有银弹。Hermes 也远远谈不上完美它还是会漏报、会误报、会遇到各种没见过的边缘情况。但只要它能帮你把评审者从“找错别字”的工作里解放出来让他们把时间花在真正需要人脑判断的问题上那这个工具就值了。
返回列表