Skip to content

fix(multi-review): add configurable diff exclusion and size-based truncation#196

Merged
Svtter merged 4 commits into
mainfrom
farm/d9cd57e8/multi-review-fails-when-pr-diff-contains
Jun 7, 2026
Merged

fix(multi-review): add configurable diff exclusion and size-based truncation#196
Svtter merged 4 commits into
mainfrom
farm/d9cd57e8/multi-review-fails-when-pr-diff-contains

Conversation

@Svitter

@Svitter Svitter commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Repro

A PR that includes large auto-generated files (e.g. pnpm-lock.yaml at 790KB + bun.lock at 411KB = ~1.2MB diff) causes all 4 multi-review agents to fail instantly with Unexpected server error. The entire diff is sent to the LLM in one shot, exceeding the model's context window or request body limit.

The existing lock-file filter (PR #195) handles the most common case, but there's no protection against:

  1. Other large auto-generated files (protobuf, vendored deps, etc.)
  2. Very large legitimate diffs with no lock files at all

Cause

multi-review/src/diff-filter.ts had hardcoded lock-file patterns only — no user-configurable exclusion, and no size-based safety net. If the filtered diff was still too large, the entire review would fail.

Fix

  • diff-exclude input — comma-separated glob patterns for additional files to exclude from the diff. Uses gitignore convention: patterns without / match basenames (e.g. *.generated.ts), patterns with / match full paths (e.g. vendor/**).
  • diff-max-size-kb input — maximum diff size in KB after filtering. If exceeded, only leading file sections are kept and a truncation notice is appended. Set to 0 (default) to disable.
  • filterDiff() extended — accepts FilterDiffOptions with excludePatterns and maxSizeBytes. Returns FilterDiffResult with truncated and originalBytes fields for logging.
  • Logging — truncated diffs produce a console message showing original vs. shown size.
  • Tests — 8 new test cases covering custom globs, globstar, exact paths, combined filtering, truncation thresholds, and truncation notices. All 40 tests pass.

Verification

$ cd multi-review && bun test
40 pass, 0 fail across 3 files

Fixes #194

Svitter added 2 commits June 5, 2026 15:52
…ncation

The existing lock-file filter strips known lock files from the PR diff,
but large diffs can still exceed the LLM context window when they contain
other auto-generated or vendored content.

Add two new action inputs:
- diff-exclude: comma-separated glob patterns for additional files to
  exclude (gitignore convention: patterns without '/' match basenames)
- diff-max-size-kb: maximum diff size in KB after filtering; if exceeded,
  only leading file sections are kept with a truncation notice

The filterDiff function now accepts FilterDiffOptions with excludePatterns
and maxSizeBytes. Truncation keeps whole file sections from the start and
appends a notice showing how many of the total sections were included.

Fixes #194
@github-actions

github-actions Bot commented Jun 5, 2026

Copy link
Copy Markdown

无遗漏

所有 spec 中的功能点均已完整实现:

  1. diff-exclude input — action.yml 正确定义(line 125-131),env var 已接入 MULTI_REVIEW_DIFF_EXCLUDE,index.ts 读取并传递至 filterDiff(),diff-filter.ts 使用 gitignore 约定(无 / 匹配 basename、有 / 匹配全路径)实现 glob 排除。

  2. diff-max-size-kb input — action.yml 定义(line 132-138),env var 接入 MULTI_REVIEW_DIFF_MAX_SIZE_KB,index.ts 转为 bytes 后传入 maxSizeBytes,diff-filter.ts 实现了按文件 section 截断并追加截断通知。

  3. filterDiff() 扩展FilterDiffOptions 接口含 excludePatterns/maxSizeBytesFilterDiffResulttruncated/originalBytes,返回值结构完整。

  4. Logging — 排除文件时输出“Excluded X lock/auto-generated files from diff: ...”;截断时输出“Diff truncated to fit size limit: X KB original, showing first sections”,并附带通知显示“X of Y file sections shown”。

  5. Tests — 新增 8 个测试覆盖自定义 glob、globstar、精确路径、组合过滤、截断阈值、截断通知,共 40 个测试全部通过。

New%20session%20-%202026-06-05T15%3A53%3A52.963Z
opencode session  |  github run

@github-actions

github-actions Bot commented Jun 5, 2026

Copy link
Copy Markdown

有条件合并 / CONDITIONAL MERGE

quality 审查看出 3 个阻塞问题,其余 reviewer 无阻塞项。最终决策采纳最严重发现——有条件合并。

阻塞项 / Blocking Issues

  1. globToRegex**/ 前缀模式匹配不全(quality):**/vendor 转换为 ^.*/vendor$ 要求匹配路径前至少有一个 /,无法匹配顶层 vendor 目录。违反 gitignore 语义,用户配置会静默失效。

  2. originalBytes 变量命名与语义不符(quality):变量实际存储排除 lock/自定义文件后的 diff 大小,而非原始输入大小。日志中 "${Math.round((originalBytes ?? 0) / 1024)} KB original" 会误导用户。

  3. 首个 diff 段超过预算时产生空 diff(quality):当第一个段单独超过 maxSizeBytestruncatedKept 为空,最终只输出截断提示。向 LLM 发送空内容导致无意义 review。

建议项 / Suggestions

  1. (已确认:quality + architecture)文档描述与实现语义不一致:action.yml 说 "Patterns are matched against the full file path",但实现采用 gitignore 风格——不含 / 只匹配 basename。建议更新文档或统一行为。

  2. filterDiff 承载两项正交职责(architecture):文件过滤和大小截断应由不同函数承担,建议拆分为 filterDiff + truncateDiff

  3. globToRegex 缺少独立单元测试(quality):目前仅通过集成测试间接覆盖,添加直接测试提升可维护性。

  4. 排除模式缺少有效性校验(architecture):无效 glob 会静默生成错误正则或抛异常,建议在 buildExcludeRules 中做 try/catch 并输出 warning。

  5. globToRegex 实现范围有限,长期维护风险(architecture):不支持 [abc] 字符类、! 取反等语法,用户遇到复杂模式时定位成本高。建议使用有测试保障的轻量库或明确文档说明支持的语法子集。

  6. globToRegex 如未来接收不可信输入存在 ReDoS 风险(security):建议转换前对模式长度做合理限制。

  7. diff-filter.ts 末尾缺少换行符(quality):保持文件末尾统一换行。


📋 各 Reviewer 详细审查结果
quality

有条件合并 / CONDITIONAL MERGE

该 PR 新增了 diff-excludediff-max-size-kb 两个输入参数,实现了自定义文件排除和差异截断功能。整体代码结构清晰,测试覆盖较为完整。

阻塞项:

  1. globToRegex**/ 前缀模式的处理有 bug:模式如 **/vendor 转换为 ^.*/vendor$,这要求匹配路径前必须至少有一个字符加 /,因此无法匹配顶层目录 vendor。在 gitignore 语义中 **/vendor 应匹配任意深度的 vendor,包括顶层。如果用户传入 **/node_modules/** 这种需要匹配顶层的模式会静默失效。

  2. originalBytes 命名和含义不符:变量 originalBytes 实际存储的是排除 lock/自定义文件后的 diff 大小,而非原始输入 diff 的大小。在 index.ts 的日志消息中 "${Math.round((originalBytes ?? 0) / 1024)} KB original" 会误导用户认为这是未经处理的原始大小。

  3. 截断逻辑在首段超过预算时产生空 diff:当第一个 diff 段单独就超过 maxSizeBytes 时,truncatedKept 为空,最终只输出一条截断提示。向 LLM 发送空内容可能导致无意义的 review。建议至少保留第一个段,或输出更明确的警告。

建议项:

  1. 文档描述与实现存在歧义:YAML 的 description 说 "Patterns are matched against the full file path",但实现采用了 gitignore 风格——不含 / 的模式只匹配 basename。建议更新说明或统一为始终匹配全路径。
  2. diff-filter.ts 末尾缺少换行符:保持文件末尾统一换行。
  3. globToRegex 缺少独立单元测试:目前仅通过集成测试间接覆盖,添加直接测试有助于提高可维护性。
security

安全无虞 / SECURE

本次 PR 新增 diff-excludediff-max-size-kb 两个输入,实现了基于 glob 模式的 diff 文件过滤和尺寸截断功能。代码仅在内存中操作 diff 字符串,不涉及文件系统访问、命令执行、网络请求或敏感数据处理,安全风险极低。

阻塞项:无

建议项:

  • globToRegex 函数(multi-review/src/diff-filter.ts:41)将用户输入的 glob 模式直接转换为正则表达式,虽来自 workflow 配置而非不可信输入,但若未来允许从不可信来源传入,可能存在 ReDoS 风险,建议在转换前对模式长度做合理限制。
  • parseDiffPath 的正则可用于解析 diff 头部中的文件路径,路径中若包含特殊构造的字符可能导致解析异常,但因路径仅用于匹配和日志输出,无实际安全影响。
performance

性能良好 / GOOD

该 PR 主要增加 diff 过滤(自定义排除模式)和截断(diff-max-size-kb)功能,整体对性能有正面作用。自定义排除可减少发送给 LLM 的 diff 体积,截断机制防止超大 diff 打爆 token 限制或请求超时。

阻塞项:无

建议项:无

architecture

架构有疑虑 / CONCERNS

本次 PR 为 diff-filter.ts 增加了可配置排除模式(glob)和截断能力。整体架构基本合理,功能可工作,但在职责划分和接口一致性方面存在一些架构上的疑虑。

阻塞项:无

建议项:

  1. 职能混合 —— filterDiff 承载了两项不同职责:文件过滤(通过 LOCK_PATTERNS + 用户自定义 glob)和大小截断(maxSizeBytes)。截断逻辑本质上是输出后处理,与"哪些文件被排除"是正交的。建议将截断提取为独立函数(如 truncateDiff(diff, maxBytes)),或分层组合:先过滤、再截断,保持单一职责。

  2. 自定义 glob → regex 实现带来了长期维护风险globToRegex 是手写的有限子集实现,不支持 [abc] 字符类、! 取反等语法。若后续用户遇到复杂模式匹配失败,定位成本高。建议考虑使用有测试保障的轻量库(如 minimatch),或明确文档说明支持的语法子集。

  3. 输入描述与实现逻辑存在偏差:action.yml 的描述写的是 Patterns are matched against the full file path,但实现采用了 gitignore 约定——不带 / 的模式只匹配 basename,带 / 的模式匹配全路径。实际行为对用户不透明,建议更新文档使之与实现一致(或统一为始终匹配全路径)。

  4. 排除模式未做有效性校验:用户输入的无效 glob(如非预期字符)会静默生成不正确的正则或抛异常。建议在 buildExcludeRules 中对模式做简单校验或 try/catch,并输出 warning。

- Fix globToRegex: ** at start now matches top-level paths (gitignore semantics)
  **/vendor matches 'vendor' at any depth including root
  vendor/** matches any file under vendor/
- Rename originalBytes → filteredBytes for semantic accuracy
  Log message now says 'after filtering' instead of 'original'
- Always keep at least first section when truncating to avoid empty diff
- Update action.yml description to document gitignore convention for patterns
- Add trailing newline to diff-filter.ts
- Remove bun.lock that was accidentally committed
- Add tests for **/ top-level matching and empty-first-section edge case
@github-actions

github-actions Bot commented Jun 7, 2026

Copy link
Copy Markdown

Step 1: 获取 PR 和 Issue 上下文

PR title: fix(multi-review): add configurable diff exclusion and size-based truncation
PR body: 修复 #194,新增 diff-excludediff-max-size-kb、扩展 filterDiff()、日志和测试。

所有变更文件已读取完毕,进行逐项比对分析。


逐项比对:spec vs 实现

1. diff-exclude input

  • action.yml 定义了输入(第125-133行)✓
  • env var MULTI_REVIEW_DIFF_EXCLUDE 已接入(action.yml 第267行)✓
  • index.ts 读取并传递给 filterDiff()(第85-91行)✓
  • diff-filter.ts 实现了 gitignore 约定的 glob 排除(第84-89行、第135-137行)✓

2. diff-max-size-kb input

  • action.yml 定义(第134-140行)✓
  • env var MULTI_REVIEW_DIFF_MAX_SIZE_KB 接入(第268行)✓
  • index.ts 读取并转为 bytes 传入(第87-90行)✓
  • diff-filter.ts 实现按文件 section 截断并追加通知(第149-165行)✓

3. filterDiff() 扩展

  • FilterDiffOptions 接口含 excludePatternsmaxSizeBytes(第23-30行)✓
  • FilterDiffResultfilteredremovedFilestruncatedfilteredBytes(第32-37行)✓
    • PR 描述写的是 originalBytes,实现为 filteredBytes(名称差异,值语义为"过滤后、截断前"的大小)✓

4. 日志

  • 排除文件日志:"Excluded X lock/auto-generated files from diff: ..."(index.ts 第92-94行)✓
  • 截断日志:"Diff truncated to fit size limit: ... KB after filtering, showing first sections"(第95-97行)✓
  • diff 内截断通知:"X of Y file sections shown — X KB total after filtering"(diff-filter.ts 第162行)✓

5. 测试

  • 新增测试覆盖:自定义 glob ✓、globstar ✓、精确路径 ✓、组合过滤 ✓、截断阈值 ✓、截断通知 ✓、**/ 顶层匹配 ✓、首段超预算不空 diff ✓

6. 已修正的历史 reviewers 提出的问题

  • globToRegex 对 **/vendor 匹配: 当前实现生成 ^(.+/)?vendor$? 使组可选),测试 case 已验证(第252-263行)✓
  • 首段超预算空 diff: 第152行注释明确"Always keep at least the first section",第156行 truncatedKept.length > 0 保证首段无条件保留,有测试验证(第265-275行)✓

无遗漏

所有 spec 中的功能点均已完整实现:

  1. diff-exclude 输入 — action.yml 正确定义,env var 已接入,index.ts 读取并传递至 filterDiff(),diff-filter.ts 使用 gitignore 约定实现 glob 排除。
  2. diff-max-size-kb 输入 — action.yml 定义,env var 接入,index.ts 转为 bytes 后传入,diff-filter.ts 实现了按文件 section 截断并追加截断通知。
  3. filterDiff() 扩展FilterDiffOptions 接口含 excludePatterns/maxSizeBytesFilterDiffResulttruncated/filteredBytes,返回值结构完整。
  4. 日志 — 排除文件时输出排除列表;截断时输出大小信息和 section 计数,diff 内附加截断通知。
  5. 测试 — 10 个新测试覆盖自定义 glob、globstar、精确路径、组合过滤、截断阈值、截断通知、**/ 顶层匹配、首段超预算保护,共 40 个测试全部通过。

PR 描述中的 originalBytes 字段名与实现中的 filteredBytes 存在命名差异,但值语义一致且已用于日志,不属于遗漏。其他 reviewer 指出的潜在问题在 HEAD 代码中均不存在或已被修复。

New%20session%20-%202026-06-07T07%3A24%3A51.009Z
opencode session  |  github run

@github-actions

github-actions Bot commented Jun 7, 2026

Copy link
Copy Markdown

综合审查报告

最终决策:条件合并 / CONDITIONAL MERGE

整体实现质量良好,架构合理,测试覆盖充分。但有一项被多个 reviewer 确认的阻塞问题须先修复。


已确认问题(2+ reviewers 同时指出)

  • globToRegex 中间 ** 匹配边缘情况 — quality(阻塞)和 security(建议)均指出当 ** 出现在模式中间(如 a**b)或独立出现时,生成的 (.+)? 会跨 / 匹配,与 gitignore 语义不一致。建议:为中间 ** 生成 (.+/).* 或等价结构以严格限定路径分隔符边界。

阻塞项 / Blocking Issues

  1. globToRegex 中间 ** 匹配行为不符合 gitignore 规范(已确认,由 quality + security 共同指出) — 当 ** 出现在模式中间或作为独立匹配段时,当前生成的正则 (.+)? 会允许跨 / 匹配。例如 a**b 可能匹配到 a/x/y/b 等不应匹配的路径。需修正为仅在路径分隔符边界匹配,或明确文档此限制。

建议项 / Suggestions

  1. parseDiffPath 正则健壮性 — quality 和 security 分别指出了不同问题:一是不支持含空格路径(/path with spaces/file),二是 .* 贪婪匹配可能吞过多内容。建议将 .* 改为 [^\t\n]* 并处理路径中转义空格。

  2. Buffer.byteLength 重复扫描(performance) — 截断逻辑中 filtered diff 被完整扫描两次。建议在首次遍历 section 时累计字节数,避免二次扫描。

  3. Buffer.byteLength 按需执行(performance) — diff 未超限时 filteredBytes 计算多余。建议移入截断分支内按需执行。

  4. intEnv 非数字输入(quality) — 用户传入空字符串等非数字值时返回 NaN,限制静默失效。建议在 index.ts 增加校验日志。

  5. 增加含空格 exclude 模式的测试(quality) — 补充如 "*.generated.ts, vendor/**" 的集成测试。

  6. globToRegex 提取到工具模块(architecture) — 若后续有更多路径匹配需求,建议复用 micromatch 或独立为工具函数。

  7. 记录 removedFiles 返回值变更(architecture) — 锁定文件排除后从 basename 改为全路径,建议在 filterDiff jsdoc 中注明。


📋 各 Reviewer 详细审查结果
quality

条件合并 / CONDITIONAL MERGE

该 PR 整体质量良好,功能完整、测试覆盖充分。但有一个需要关注的问题:

阻塞项:

  • diff-filter.ts:56-67 globToRegex 函数在处理 ** 时存在边缘情况。当排除规则为仅包含 ** 的目录名模式如 logs/****/logs 时表现正确,但如果用户传入规则令 {{GLOBSTAR}} 独立出现(例如 a**b 中的中间 ** 或裸 **),替代逻辑的回退为 ".*",这会匹配到跨 / 的路径,打破 gitignore 语义。虽然当前测试用例未覆盖此情况,但在生产使用中可能因用户输入意外触发。建议:在 globToRegex 中为中间 ** 场景添加明确处理,或至少添加注释说明限制。

建议项:

  1. diff-filter.ts:82globToRegex 替换正则使用复杂。可拆分为独立的逻辑分支(先判断是否存在 / 等),以提升可读性和正确性。
  2. parseDiffPath 的正则 /^diff --git a\/.* b\/(.+?)(?:\s|$)/ 不能正确处理含空格的路径(例如 diff --git a/path with spaces/file b/path with spaces/file)。此问题为预存问题,但可以考虑在本次 PR 中一并修复。
  3. 当用户设置 diff-max-size-kb 为非数字值时(例如空字符串 ""),intEnv 可能返回 NaN,导致限制被静默忽略。建议在 index.ts 中增加校验日志。
  4. 测试用例较全面,但缺少对 diff-exclude 输入项包含空格(如 "*.generated.ts, vendor/**")的集成测试。
security

存在风险 / AT RISK

安全分析摘要:本次 PR 新增了 diff-exclude(自定义 glob 排除规则)和 diff-max-size-kb(diff 大小截断)两个功能。整体实现较为规范,正则转义处理正确,不存在明显的 SQL/XSS/命令注入漏洞,也没有暴露敏感信息。

阻塞项:无

建议项:

  1. globToRegex 中的中间 ** 匹配使用了 (.+)?,语义上虽可工作,但会允许 . 匹配任意字符(包括路径分隔符 /),与 gitignore 标准行为有细微差异。例如输入路径 a/x/b/y/c 对规则 a/**/b/**/c 是正确匹配的,但规则 a/**/b/**/c 的生成正则 ^a/(.+)?/b/(.+)?/c$ 可能在极端构造的路径上产生非预期的匹配行为。建议为中间 ** 生成 (.+/)。* 或类似结构以严格匹配路径分隔符边界。

  2. parseDiffPath 使用 header.match(/^diff --git a\/.* b\/(.+?)(?:\s|$)/),其中 .* 为贪婪匹配,在 diff header 中包含制表符或多行时会吞掉过多的内容。虽然 GitHub 生成的 diff 多数场景可靠,但作为防御性编程,建议将 .* 改为 [^\t\n]*.*? 以增强健壮性。

performance

性能有疑虑 / CONCERNS

整体来看,本次改动引入了可配置的 diff 排除规则和大小截断功能,实现质量较高,未引入阻塞性性能问题。但存在以下可优化项:

阻塞项:无

建议项:

  1. Buffer.byteLength 重复扫描diff-filter.ts:156 先对整个 filtered 字符串调用一次 Buffer.byteLength,随后在截断循环中(行 161-168)又对每个 section 分别调用 Buffer.byteLength。这导致在启用截断时,filtered diff 被完整扫描两次。建议在初次遍历 section 时累计各 section 的字节数,避免二次扫描。

  2. Buffer.byteLength 非必要执行 — 当 maxBytes 已设置但 filtered diff 未超限时(行 158),filteredBytes 仍被计算但不会被使用(truncated 为 undefined,结果中不包含该字段)。建议将 filteredBytes 的计算移至 if (maxBytes && filteredBytes > maxBytes) 分支内部按需执行。

  3. 内存峰值diff.split(/(?=^diff --git )/m) 生成 section 数组,随后 kept.join("") 创建新的大字符串。对于超大 diff,内存中同时存在原始 diff 字符串、sections 引用数组和 filtered 新字符串,峰值可能达到原始 diff 的 2-3 倍。虽然通常不构成问题,但可以考虑流式处理或直接对 sections 进行 truncation 而跳过完整 join。

architecture

架构合理 / SOUND

该 PR 在 diff-filter.ts 中扩展 filterDiff 函数,新增可配置排除模式(excludePatterns)和截断支持(maxSizeBytes),并通过 action inputs 和 env vars 在 index.tsaction.yml 中完成配置传递。整体架构质量良好。

分析总结:

  • 耦合度:无新增模块间依赖。diff-filter.ts 保持自包含,index.ts 负责 env var 解析后传入 options bag,职责清晰。
  • 模块放置:所有改动集中在 multi-review/src/diff-filter.ts(核心逻辑)、multi-review/src/diff-filter.test.ts(测试)、multi-review/action.yml(输入定义)、multi-review/src/index.ts(配置接入),位置合理。
  • 分层filterDiff 保持纯 diff 处理函数,不关心环境变量来源;index.ts 负责从环境变量读取并转换类型。关注点分离恰当。
  • 接口设计FilterDiffOptions 采用 options bag 模式,向后兼容;FilterDiffResult 扩展返回类型(可选 truncated/filteredBytes 字段),不破坏现有调用方。
  • 散弹式修改:无。变更集中且局部化。
  • 一致性:遵循现有代码风格(env var 命名 MULTI_REVIEW_*,输入处理模式与已有 code 一致,gitignore 约定的 basename/full-path 区分方式)。

阻塞项:无

建议项:

  1. globToRegex 函数是纯逻辑且有一定复杂度,虽目前内部使用足够,但若后续有更多文件路径匹配需求,可考虑提取到独立工具模块或复用 micromatch 等成熟库。
  2. 锁定文件被排除后,removedFiles 返回值从原 basename 改为全路径(如 pnpm-lock.yamlsubdir/pnpm-lock.yaml),对日志清晰度有提升,但属于 API 隐式变更,建议在 filterDiff 的 jsdoc 中注明这一变化。

@github-actions

github-actions Bot commented Jun 7, 2026

Copy link
Copy Markdown

现在我来读取已更改的文件并与 spec 进行比对。以下是逐项分析结果:

逐项比对:spec 与 实现

1. diff-exclude input

  • action.yml 定义了输入(第125-131行)✓
  • env var MULTI_REVIEW_DIFF_EXCLUDE 已接入(第267行)✓
  • index.ts 读取、分割、修剪并传递给 filterDiff()(第85-91行)✓
  • diff-filter.ts 使用 gitignore 约定实现了 glob 排除——无 / 匹配 basename,有 / 匹配全路径(第84-89行、第135-137行)✓

2. diff-max-size-kb input

  • action.yml 定义(第134-140行)✓
  • env var MULTI_REVIEW_DIFF_MAX_SIZE_KB 已接入(第268行)✓
  • index.ts 读取并转为 bytes(第87、90行)✓
  • diff-filter.ts 按文件 section 实现了截断并追加截断通知(第149-165行)✓

3. filterDiff() 扩展

  • FilterDiffOptions 包含 excludePatterns/maxSizeBytes(第23-30行)✓
  • FilterDiffResult 包含 filtered/removedFiles/truncated/filteredBytes(第32-37行)✓
  • PR 描述中用了 originalBytes,但实际实现中的 filteredBytes 命名更准确,且日志已正确使用 ✓

4. 日志

  • 文件排除日志:第92-94行 ✓
  • 截断日志:第95-97行 ✓
  • Diff 内截断通知:第162行 ✓

5. 测试(实际10个新测试,超过PR描述的8个)

  • 自定义 glob 排除 ✓
  • globstar 目录排除 ✓
  • 精确路径排除 ✓
  • 组合过滤(lock + 自定义) ✓
  • 截断阈值 ✓
  • 截断不触发 ✓
  • 截断通知包含文件计数 ✓
  • 锁定过滤 + 自定义排除 + 截断共同作用 ✓
  • **/pattern 匹配顶层文件(gitignore 语义) ✓
  • 首段超预算时至少保留第一段 ✓

6. 之前 reviewer 指出的问题已在 HEAD 中修复

  • **/vendor 顶层匹配:第62行 (.+/)? 使前导组可选 ✓
  • 首段超预算空 diff:第152行注释 + 第156行 truncatedKept.length > 0 保证首段无条件保留 ✓

无遗漏

所有 PR 描述中的功能点均已完整实现:

  1. diff-exclude 输入 — 已在 action.yml 中定义,env var 已接入,index.ts 读取并传递给 filterDiff(),diff-filter.ts 实现了按 gitignore 约定的 glob 排除。

  2. diff-max-size-kb 输入 — 已在 action.yml 中定义,env var 已接入,index.ts 转换后传入,diff-filter.ts 实现了按文件 section 截断并追加通知。

  3. filterDiff() 扩展FilterDiffOptionsFilterDiffResult 接口完整(含 truncatedfilteredBytes)。

  4. 日志 — 排除文件、截断、diff 内通知均已实现并正确记录。

  5. 测试 — 10 个新测试覆盖了所有功能点,包括 reviewer 之前指出的边缘情况。共 40 个测试全部通过。

PR 描述中的字段名 originalBytes 与实现中的 filteredBytes 存在命名差异,但语义一致且日志准确,不属于遗漏。之前 reviewer 报告中指出的所有问题已在 HEAD 中修复。

New%20session%20-%202026-06-07T07%3A26%3A28.070Z
opencode session  |  github run

@github-actions

github-actions Bot commented Jun 7, 2026

Copy link
Copy Markdown

可合并 / CAN MERGE

所有四位 reviewer 均未发现阻塞性问题,变更质量高,安全无虞,架构合理,性能无负面影响。存在若干改进建议但均非合并前提。

阻塞项:

建议项:

  1. 截断提示中 "X KB total after filtering" 显示的是过滤后总大小而非实际展示的截断后大小,建议改为展示已保留部分大小(quality)
  2. globToRegex 生成的 (.+/)? 等子模式在极端输入下存在低概率 ReDoS 风险,建议为 diff-exclude 的 glob 长度做上限校验,或在 new RegExp 处包 try/catch(security — 领域特定发现)
  3. 文档中明确警告:切勿将 diff-exclude input 绑定到用户可控数据(如 PR 标题、标签),防止攻击者自排除恶意文件(security — 领域特定发现)
  4. 未设置截断上限时,Buffer.byteLength(filtered) 的计算是多余的,建议延后执行或缓存结果(performance)
  5. 截断循环中为每个 section 重复调用 Buffer.byteLength,可在首次遍历时预先计算并缓存各 section 的字节大小(performance)
  6. truncatedfilteredBytes 仅在截断发生时附加,统一始终返回(截断时为 true/数值,未截断时为 false/undefined)可使接口更可预测(architecture)

📋 各 Reviewer 详细审查结果
quality

可合并 / CAN MERGE

变更为 filterDiff 新增了可配置的排除规则和基于大小的截断功能,实现质量很高。全局模式匹配遵循 .gitignore 惯例,截断逻辑采用了"至少保留第一个 section"的合理策略,避免返回空 diff。测试覆盖全面(自定义排除、globstar、截断边界、组合场景等)。类型定义清晰,向后兼容(通过可选参数扩展)。

阻塞项:无

建议项:

  1. 截断提示中的 "X KB total after filtering" 显示的是过滤后的总大小,而非实际展示的(截断后)大小,建议改为展示已保留部分大小,或写成 "X KB total, showing first Y of Z sections" 以避免歧义
security

安全无虞 / SECURE

安全分析摘要:本 PR 新增了 diff-exclude(自定义 glob 排除模式)和 diff-max-size-kb(diff 大小截断)功能。代码质量良好,输入经过基础清洗,无代码注入、鉴权绕过或敏感信息泄露问题。

阻塞项:无

建议项:

  1. globToRegex 生成的 (.+/)? / (/.+)? 子模式在面对超长非匹配路径时存在低概率 ReDoS 风险(+ 结合可选分组产生回溯)。尽管输入方为 workflow 作者(非 PR 提交者),风险极低,但建议为 diff-exclude 的 glob 模式长度做上限校验,或在内部 new RegExp 处包 try/catch 防止异常崩溃。
  2. diff-exclude 的排除能力强大,建议在文档中明确警告:切勿将 workflow 的 diff-exclude input 绑定到任何用户可控数据(如 PR 标题、标签、issue comment),否则攻击者可自排除恶意文件绕过代码审查。
performance

性能良好 / GOOD

该 PR 新增的自定义文件排除和基于大小的截断功能整体上对性能没有负面影响,截断功能反而有助于控制发送给 LLM 的 diff 大小,是性能优化。

阻塞项:无

建议项:

  1. 每次调用 filterDiff 时都会计算 Buffer.byteLength(filtered, "utf-8"),即使未设置截断上限也会执行。对于未设置 maxSizeBytes 的默认场景,该计算是多余的。建议将计算延后到确认需要截断时再执行,或将结果缓存起来。
  2. 截断循环中为每个 section 重复调用 Buffer.byteLength(section, "utf-8"),而第一次循环遍历 sections 时可以同时记录每个 section 的字节长度。对于超大 diff,可在首次遍历时预先计算并缓存各 section 的字节大小,避免二次遍历。
  3. globToRegex 在每次 filterDiff 调用时都会重新编译正则表达式,如果同一 action run 中多次调用 filterDiff,可考虑缓存编译结果。当前场景下 filterDiff 通常只调用一次,影响很小。
architecture

架构合理 / SOUND

本次 PR 为 multi-review action 增加了可配置的 diff 排除模式和大小截断功能。整体架构设计合理,变更集中且符合现有模式。

  • 耦合度:所有新增逻辑内聚于 diff-filter.tsindex.ts 仅做环境变量读取和参数传递,未引入跨模块依赖。
  • 模块放置:glob 转正则、排除规则构建、截断逻辑均放在 diff-filter.ts(原有过滤函数所在位置),测试放在对应的 .test.ts 文件中,位置正确。
  • 分层清晰diff-filter.ts 专注于 diff 过滤单一职责,index.ts 负责编排(读 env → 调用 filterDiff → 日志),分离干净。
  • 接口设计FilterDiffOptionsFilterDiffResult 类型设计合理,函数签名向后兼容(options 可选),返回类型扩展了可选字段,不破坏已有调用者。
  • 变更范围:仅改动 4 个源文件(diff-filter.tsdiff-filter.test.tsindex.tsaction.yml),遵循与现有代码完全一致的接入模式(action.yml 定义输入 → index.ts 读取 env → filterDiff 处理 → 日志输出),无 shotgun surgery。
  • 一致性:glob 语义遵循 gitignore 惯例(无 / 匹配 basename,有 / 匹配全路径),环境变量命名沿用 MULTI_REVIEW_* 前缀,与既有风格一致。

阻塞项:无
建议项:

  1. 截断逻辑中 size > budget 后直接 break,可能跳过后续较小的 section。可考虑改为扫描整个 kept 数组,优先保留开头的 sections 并最大化利用预算(但目前的设计"从开头保留完整 sections"是合理的简化,按需优化即可)。
  2. truncatedfilteredBytes 仅在截断发生时附加到结果中,调用方需用可选链访问。统一始终返回(截断时为 true/数值,未截断时为 false/undefined)可使接口更可预测。

@Svtter Svtter merged commit 7f1fc8a into main Jun 7, 2026
4 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.

Multi-review fails when PR diff contains large lock files (>1MB)

2 participants