refactor: improve hash regex, parseExtraEnv API, test assertions

[Svtter]

Summary

Addresses review suggestions from PR #177.

Changes

1. escapeHashReferences regex broadened (multi-review/src/platform.ts)

   • Lookbehind: added （ (fullwidth paren), ", ', `, <
   • Lookahead: added ） (fullwidth paren), ", ', `, ,
   • Covers cases like (#1, #2), [#1], #1、, quotes/backticks

2. parseExtraEnv() refactored (multi-review/src/platform.ts + index.ts)

   • No longer calls process.exit(1) internally
   • Returns ExtraEnvResult { blockedKeys: string[] } instead
   • Exit handling moved to caller in index.ts
   • Easier to test and reuse

3. Strengthened test assertion (tests/test_all.py)

   • test_extra_env_deduplicates_blocked_keys now asserts the exact
     summary message "blocked 1 disallowed key override(s): MODEL" instead
     of just counting occurrences

[github-actions]

发现遗漏

分析摘要

PR 声明了三项重构变更。已逐项核对最新代码：

1. 正则扩展 — multi-review/src/platform.ts:163 的 HASH_NUM_RE 已包含 spec 列出的所有新增字符（lookbehind：（、"'、<；lookahead：）、"'、,），并保留了原有的中文标点。multi-review/dist/index.cjs:5263 与源代码一致。
2. parseExtraEnv() 重构 — 函数内已无 process.exit(1)；返回 ExtraEnvResult { blockedKeys: string[] }（platform.ts:604-643）；调用方 index.ts:45-48 检查 envResult.blockedKeys.length > 0 后 return 1。dist 文件同步。
3. 测试断言强化 — tests/test_all.py:767 改为 assertIn("blocked 1 sensitive key override(s): MODEL", result.stderr)，与 PR 描述一致。

已发现的遗漏

MEDIUM

强化的测试断言与实际代码输出不匹配，测试将运行失败

• 缺失项：测试字符串 "blocked 1 sensitive key override(s): MODEL" 与 platform.ts:641 实际打印的 extra-env: blocked 1 disallowed key override(s): MODEL 不一致（关键字 sensitive vs disallowed），assertIn 子串匹配将失败。
• 影响：test_extra_env_deduplicates_blocked_keys 这条强化的测试无法通过 PR 描述中"asserts the exact summary message"的目标——sensitive 在代码输出中从未出现。
• 涉及 spec 段落：PR body 第 3 项（"test_extra_env_deduplicates_blocked_keys now asserts the exact summary message 'blocked 1 sensitive key override(s): MODEL'"）。
• 修复方向（仅说明，不修改）：二选一
  1. 将 platform.ts:641 的措辞由 disallowed key override(s) 改为 sensitive key override(s)，并同步 dist/index.cjs:5682。
  2. 或者将 tests/test_all.py:767 的断言文本改为 "blocked 1 disallowed key override(s): MODEL"，与现有代码输出一致。
• 关联位置：multi-review/src/platform.ts:641、multi-review/dist/index.cjs:5682、tests/test_all.py:767。

opencode session  |  github run

[github-actions]

最终决策：不可合并

总结

PR 整体在安全、架构与性能方面是良性重构（escapeHashReferences 分段化、parseExtraEnv 解耦进程控制、构建产物入库）。但存在 1 项 CI 必败的阻塞项：测试断言关键字与代码实际输出不一致。

阻塞项

• tests/test_all.py:767 断言与代码输出不匹配：assertIn("blocked 1 sensitive key override(s): MODEL", result.stderr)，但 multi-review/src/platform.ts:641（同步 dist/index.cjs:5682）实际输出 extra-env: blocked N disallowed key override(s): ...。关键字 sensitive ≠ disallowed，assertIn 必失败，CI 红。修复方向二选一：
  • 改测试：assertIn("blocked 1 disallowed key override(s): MODEL", result.stderr)
  • 改代码：把 disallowed 统一为 sensitive（注意 MULTI_REVIEW_* 前缀与敏感键共用同一段输出，需统一定义）。

建议项

已确认（≥2 reviewer 同意）：

1. dist/index.cjs 手工同步 → 自动化构建（quality / security / architecture 共识）：建议在 CI 加 tsc 构建步骤并 commit dist 改为自动产物，或加 npm run build 后比较 dist/ 与 src/ 一致性检查。
2. escapeHashReferences 分段逻辑的扫描效率（quality / performance 共识）：对非 code 段每段单独跑 INLINE_CODE_RE.matchAll 与 HASH_NUM_RE.replace，可合并为单次遍历或加入 early-return；对极短文本可在入口加快速判断。
3. HASH_NUM_RE 边界场景覆盖（quality / security / architecture 共识）：lookbehind 仍不含字母（如 #1abc）、HTML 标签 <a href="#1">、Markdown 链接 [x](#1)、（#1） 混合标点等边角情况，建议补充单元测试防止回归。

单一意见（仅 1 reviewer 提出）：
4. 错误日志将 MULTI_REVIEW_* 前缀与敏感键混在同一 disallowed 桶中，建议在 parseExtraEnv 内分桶（prefixBlocked / sensitiveBlocked）后分别打印，便于运维定位（quality）。
5. escapeHashReferences 对未闭合的反引号/fence 会误转义其后内容，建议在 docstring 与用户提示中明确该限制（security）。
6. parseExtraEnv 在无 blocked key 路径上仍执行 sort() 与空数组分配，可在空集时直接 return { blockedKeys: [] }（performance）。
7. parseExtraEnv 日志副作用（console.log annotations / console.error summary）未结构化进返回值，建议将 annotations 一并返回以解耦输出位置（architecture）。

---

📋 各 Reviewer 详细审查结果

quality

不可合并

本 PR 做了三件事：(1) 扩展 HASH_NUM_RE 正则覆盖全角括号、引号、反引号、尖括号等场景，并重构 escapeHashReferences 跳过 fenced/inline code 块；(2) 把 parseExtraEnv 从「直接 process.exit(1)」改为「返回 ExtraEnvResult」由 main() 决定退出码；(3) 收紧 test_extra_env_deduplicates_blocked_keys 的断言。前两件事整体质量不错，但测试与代码输出不一致，会导致 CI 失败。

阻塞项：

• tests/test_all.py:767 断言 self.assertIn("blocked 1 sensitive key override(s): MODEL", result.stderr)，但 multi-review/src/platform.ts:641（同步 dist/index.cjs:5682）实际写入 stderr 的是 extra-env: blocked N disallowed key override(s): ...。"sensitive" ≠ "disallowed"，assertIn 必然失败。需二选一：把测试期望改为 "blocked 1 disallowed key override(s): MODEL"，或把代码输出的关键字改为 sensitive（注意 MULTI_REVIEW_* 前缀被阻止时也用同一句话，需统一定义）。

建议项：

• 错误日志用 disallowed 过于笼统，把「MULTI_REVIEW_ 前缀」与「敏感键」混在一起，运维定位不便。可以在 parseExtraEnv 内分桶（如 prefixBlocked、sensitiveBlocked）后分别打印。
• HASH_NUM_RE 的 lookahead 仍不含字母，对 #1abc 这类 GitHub 实际会截断为 #1 自动链接的场景不转义（既有问题，非本次引入，但建议后续单独处理）。
• escapeSegment 内部为「非 code 段」中的每段都新建一个 INLINE_CODE_RE.matchAll 迭代，对没有 inline code 的输入仍会跑一遍空循环，行为正确但可加 early-return 减少噪音。
• 平台层在 escapeHashReferences 的注释里仍然写着「prompt-layer instructions are merely hints」，但本次又把 HASH_AVOID_* 提示从 inline 提到模块级常量 (HASH_AVOID_ZH/EN)，文档/实现一致性 OK，无需改动；只是提请确认 reviewer persona 的 prompt 也会同步使用新常量以避免漂移。
• dist/index.cjs 此次手工同步进 PR，建议在 CI 中加 tsc 构建步骤并 commit dist 改为自动产物，避免日后 src/dist 漂移。

security

安全无虞

本 PR 主要为代码质量与功能完善，未引入新的攻击面，安全性有改进。

分析概述：

• escapeHashReferences 改写为分段处理逻辑，先剥离 fenced code（```）与 inline code（`）区间，再对其余文本应用 HASH_NUM_RE。这实际上提升了安全性：原先的暴力替换可能破坏代码块中的合法 #N（如示例代码、配置文件片段），现在会保留原始语义。
• HASH_NUM_RE 正则扩展了 lookbehind/lookahead 字符集（新增 <、（、"、'、`、>、）、,、.、，、。 等），目的是更准确识别 GitHub 自动链接 #N 的边界场景。字符类长度有限，无嵌套量词，不存在 ReDoS 风险。
• parseExtraEnv 重构为返回 ExtraEnvResult，由 main() 判断 blockedKeys.length 并返回 1（与原 process.exit(1) 等效）。tests/test_all.py 中 test_extra_env_deduplicates_blocked_keys 仍断言 returncode != 0，证明安全语义被保留。原先的 process.exit(1) 在错误处理链中可能绕过资源清理，新写法更稳健。
• buildLangInstruction 仅为抽取重复字符串，无安全影响。
• 正则中零宽空格（\u200B）注入到 GitHub 评论的 body 中，是反 XSS/反误转换的必要处理，不构成新风险（body 本身由 action 控制，不来自不可信用户输入经此函数直接渲染）。

阻塞项：无

建议项：

1. multi-review/dist/index.cjs 与 multi-review/src/index.ts、platform.ts 同步修改。建议在 CI 中加入 dist/ 与 src/ 一致性检查（npm run build 后比较），避免手工维护 dist 导致偏移；如已存在该流程可在 PR 描述中确认。
2. escapeHashReferences 的代码注释已声明 "Does NOT handle unclosed fences/backticks"，对未闭合的反引号或代码块，正则仍会作用于其后的内容，可能误转义。建议在文档或函数 docstring 中明确该限制，并在用户输入提示中提示避免未闭合反引号。
3. HASH_NUM_RE 的 lookbehind 现已包含 `，配合 INLINE_CODE_RE 的跳过逻辑可避免误命中；建议在 platform.ts 增加单元测试覆盖以下边角情况以防止未来回归：HTML 实体中的 #1、Markdown 链接 [x](#1)、混合中英文标点的列表项（如 （#1））等。

performance

性能有疑虑

性能分析：本次 PR 主要是正确性改进（在 fenced/inline code 中跳过 #N 转义）以及将 process.exit(1) 改为返回值以便测试。整体性能开销可控，但有几处值得关注：

阻塞项：无

建议项：

1. multi-review/src/platform.ts:184-198 中 escapeHashReferences 对每个非 code 段落分别调用 text.slice(...).replace(HASH_NUM_RE, ...)，如果评论中有大量 inline code 片段，会触发多次独立的 regex 扫描。鉴于 HASH_NUM_RE 现已包含更复杂的 Unicode 字符 lookbehind（[\s(\[{<（"'>:，、：]`），单次匹配成本上升。建议合并为一次遍历（手工扫描或单一 multi-pass 状态机），但 PR 评论文本通常较小，影响有限。
2. parseExtraEnv 改动后即使在无 blocked key 的常见路径上也无条件执行 [...blockedKeys].sort() 并返回空数组，会产生微小但无意义的分配。可在空集时直接 return { blockedKeys: [] } 跳过排序。
3. HASH_NUM_RE 升级后的 Unicode lookbehind 在长文本（极端 PR 描述）下匹配开销高于旧版正则；建议在 escapeHashReferences 入口对长度做一次快速判断（如 < 4 字节直接返回原文本），避免空调用开销。
4. buildLangInstruction 将 HASH_AVOID_ZH / HASH_AVOID_EN 提升为模块级常量是轻微正向优化，无问题。

architecture

架构合理

架构分析总结

本次 PR 是一次范围明确、局部化的重构，涉及三个相互独立但同源的关注点：

1. 解耦 parseExtraEnv 与进程控制：src/platform.ts:608 将 void 返回改为 ExtraEnvResult，原 process.exit(1) 上移到 src/index.ts:46-48 由 main() 统一处理退出码。这符合 "工具函数不应控制进程生命周期" 的分层原则，与 index.ts 中 "No reviewers configured"、"All reviewers failed" 等同类失败路径的退出码处理方式一致。
2. escapeHashReferences 分层分段化：src/platform.ts:182-209 引入 escapeSegment 私有辅助函数，先按 fenced code 切分、再按 inline code 切分，regex 仅作用于纯文本片段。代码块保护与作者注释 multi-review/src/platform.ts:151-159（"platform layer is the authoritative safeguard"）的职责定位吻合。
3. 测试断言收紧：tests/test_all.py:760-767 由模糊的 count("blocked") <= 1 改为对 summary 文本的精确匹配，反映实现合约。

阻塞项：无

建议项

• parseExtraEnv 仍带有日志副作用（console.log 输出 ::error::/::warning:: annotations、console.error 输出 summary），而返回结构仅暴露 blockedKeys: string[]。若未来需要在测试中抑制或重定向这些输出，函数签名会再次需要扩展。可考虑把 annotations 与 summary 也作为返回值的一部分（如 { blockedKeys, annotations }），让 index.ts 决定输出位置。当前改动未触及此层，故不阻断。
• dist/index.cjs 与 src/ 一同修改是必要的（仓库将构建产物入库），但请确认 CI 中有从 src/ 重新构建 dist/ 的步骤，避免 dist 与 src 漂移。
• HASH_NUM_RE 中新增的 < 与 < 配对符在 HTML 标签场景下可能误判（例如 <a href="#1">），但因后续有 fenced/inline code 切分保护，影响有限，仍属可接受边界。

[github-actions]

发现遗漏

分析摘要

本 PR 描述了 3 项变更：扩展 HASH_NUM_RE 正则、重构 parseExtraEnv、强化测试断言。已逐项核对最新代码：

1. 正则扩展 — multi-review/src/platform.ts:163 的 HASH_NUM_RE 已包含 PR 描述中列出的所有新增字符（lookbehind 多了 （"'<、lookahead 多了 ）"',），原字符集保留完整。✓
2. parseExtraEnv() 重构 — 函数内无 process.exit(1)（grep 全文件仅匹配到函数定义本身）；返回 ExtraEnvResult { blockedKeys: string[] }（platform.ts:605-643）；index.ts:45-48 改为 if (envResult.blockedKeys.length > 0) return 1;。✓
3. 测试断言强化 — tests/test_all.py:767 已从模糊的 count("blocked") 改为对 summary 文本的精确 assertIn，形式上与 spec 一致，但断言文本与 PR 描述指定的字面量不一致，详见下文。

已发现的遗漏

LOW

PR 描述（spec）与实际测试断言文本不一致

• 缺失项：PR body 第 3 项明确指出 test_extra_env_deduplicates_blocked_keys now asserts the exact summary message "blocked 1 sensitive key override(s): MODEL"。但 tests/test_all.py:767 实际写入的是 self.assertIn("blocked 1 disallowed key override(s): MODEL", result.stderr)，关键字为 disallowed 而非 spec 所述的 sensitive。
• 影响：测试本身能通过（platform.ts:642 与 dist/index.cjs:5682 实际输出 disallowed，与测试断言一致），故 CI 不会变红；但 PR 描述作为本改动的"spec"，所引用的字面量在代码库中不存在（grep 全仓 sensitive key override 零命中，disallowed key override 命中 4 处：platform.ts:642、run-github-opencode.py:521、dist/index.cjs:5682、test_all.py:767）。任何依赖 PR 描述回溯变更的 reader 都会发现两者对不上。
• spec 段落：PR body 第 3 项（"test_extra_env_deduplicates_blocked_keys now asserts the exact summary message "blocked 1 sensitive key override(s): MODEL""）。
• 建议修复方向（仅说明，不修改）：二选一
  1. 将 tests/test_all.py:767 的断言文本统一为 spec 所述的 "blocked 1 sensitive key override(s): MODEL"，并把 platform.ts:642 / dist/index.cjs:5682 / run-github-opencode.py:521 的 disallowed 全部改为 sensitive（注意 MULTI_REVIEW_* 前缀与敏感键共用同一段 summary 输出，需统一定义）。
  2. 或把 PR 描述中的字面量改为 "blocked 1 disallowed key override(s): MODEL"，与当前代码输出一致。
• 关联位置：multi-review/src/platform.ts:642、multi-review/dist/index.cjs:5682、github-run-opencode/run-github-opencode.py:521、tests/test_all.py:767、PR body 第 3 项。

opencode session  |  github run

[github-actions]

可合并

本次 PR 涵盖三处变更：parseExtraEnv 从内部 process.exit(1) 改为返回结构化结果（architecture/security 一致评价为合理的解耦修正）；HASH_NUM_RE 扩展前后缀字符集并新增 fenced/inline code 防护正则（security 评价为 A03 输入净化增强）；"sensitive" → "disallowed" 术语跨 run-github-opencode.py / platform.ts / tests/test_all.py 三处统一。三个有效 reviewer（security、performance、architecture）均未发现阻塞项，质量无回退。

阻塞项：无

建议项：

1. （architecture 独有，security 弱认同）parseExtraEnv 同时承担解析、环境副作用与 stderr 输出，违反 CQS。建议拆分为 parseExtraEnvSpec()（纯函数，返回 { allowed, blocked }）与 applyExtraEnv(spec)（副作用），便于测试与未来 dry-run。同步将 console.error(...blocked...) 上移到 main()，与 return 1 同栈帧，避免「日志说失败、进程却返回 0」的不一致窗口。
2. （architecture 独有）parseExtraEnv 的抽象层与 platform.ts 中其他职责（GitHub/Gitea 评论抓取、markdown 转义）不一致，长期可迁出至独立的 config.ts。
3. （security 独有）HASH_NUM_RE 字符集已较复杂，建议在函数 JSDoc 补充一个未关闭 fence 边界情况的最小示例，方便维护者理解。
4. （security 独有）parseExtraEnv 返回的 blockedKeys 当前仅用于长度早返，后续若需结构化 CI 输出（JSON 行）可保留为扩展点。

---

📋 各 Reviewer 详细审查结果

quality

（审查失败: fetch failed）

security

安全无虞

本次 PR 的安全分析：

• 输入验证与净化：escapeHashReferences 的正则扩展（增加 <、（、"、'、` 等前后缀字符）属于防御性增强，更多场景会被正确转义，避免被 GitHub 自动识别为 issue/PR 引用。FENCED_CODE_RE/INLINE_CODE_RE 的引入防止误伤代码块内的 #N，降低了"误转义导致信息泄露"或"漏转义导致意外引用"的风险。
• 认证与授权：未涉及。
• 注入漏洞：无新增。parseExtraEnv 的重构仅改变返回值（void → ExtraEnvResult），未改变对敏感键的拦截逻辑，也未改变 process.exit(1) 的失败行为。blockedKeys 在合并前已经过 Set 去重并排序，日志输出仍受控。
• 敏感数据暴露：未引入新日志泄露；错误信息只打印键名，不打印值。sensitive → disallowed 仅为措辞调整，不影响安全模型。
• 依赖与 API：未新增依赖。
• OWASP Top 10：本次变更主要涉及 A03（注入）的输入净化增强与 A09（日志）层面的最小化披露，整体方向是更安全。

阻塞项：无

建议项：

• HASH_NUM_RE 的前瞻/后顾字符集已较为复杂，文档可考虑补充一两个示例覆盖未关闭 fence 的边界情况（README/PR 描述中已注明，但函数 JSDoc 仅说 "unclosed markers will be treated as code"，可考虑加一个最小示例以便未来维护者理解）。
• parseExtraEnv 返回的 blockedKeys 仅在 main() 内使用一次后即丢弃，可考虑让 index.ts 直接用其长度做早返判断（当前已如此），但若后续需在退出时打印更结构化信息（如 JSON 行供 CI 解析），可保留以备扩展。当前实现无问题，仅为后续可演进点。

performance

性能良好

本次 PR 主要是文案修正（"sensitive" → "disallowed"）和功能性改动（解析 extra-env 后改为返回结果而非 process.exit，扩展 hash 转义的正则字符类以覆盖更多标点）。整体无明显的性能回退：

• parseExtraEnv 改用返回值而非 process.exit(1)，仍是一次性调用，新增的 [...blockedKeys].sort() 在空集情况下是 O(1)，无影响。
• escapeHashReferences 的字符类（前置：[\s(\[{<（"'>:，、：]，后置：[\s)]}>）"',.!?;，。！？、：]) 各增加几个字符，属于线性增长，对单次匹配影响可忽略。
• 新增的 buildLangInstruction 只在 loadReviewers 中调用一次，复杂度 O(1)。
• text.matchAll 对 FENCED_CODE_RE / INLINE_CODE_RE 的两遍扫描与之前实现一致，未引入额外复杂度。

阻塞项：无
建议项：无

architecture

架构有疑虑

本次 PR 主要是将 parseExtraEnv 从「内部 process.exit(1)」改为「返回结构化结果，由调用方决定退出码」，同时把 HASH_NUM_RE 上下文识别扩展到更多标点，并统一了 "sensitive" → "disallowed" 的术语。从架构角度分析：

耦合：parseExtraEnv 解耦了「解析/校验」与「进程退出控制」，调用方 main() 现在能拿到 blockedKeys 自行决定返回码，是一次合理的解耦。原本 process.exit(1) 写在被调用函数内，使得该函数无法在测试中或在其他上下文中复用，新签名修正了这一点。

接口设计：ExtraEnvResult { blockedKeys: string[] } 是一个清晰的、只读的契约。但该函数同时承担了三个职责：① 解析字符串 ② 校验敏感键 ③ 修改 process.env 副作用 + 写 stderr。这属于命令-查询分离（CQS）的轻度违反——返回值是「查询」，但函数体里仍有「命令」式的副作用，未来如果想在不写环境的情况下做 dry-run 校验，会被迫重写。

模块归属：parseExtraEnv 放在 platform.ts 中，但本质是「环境变量/配置加载」职责，与 platform.ts 中其他 Gitea/GitHub 评论抓取、hash 转义等「平台适配」职责并不属于同一抽象层。从既有代码组织看，环境变量加载更贴近 index.ts 的 bootstrap 阶段，或单独抽到 config.ts。这是历史遗留的归属问题，本次 PR 没有恶化它。

一致性：

• "sensitive" → "disallowed" 跨 run-github-opencode.py、multi-review/src/platform.ts、tests/test_all.py 三处统一，这是好的术语收敛。
• dist/index.cjs 与 src/index.ts、src/platform.ts 同步重建，符合既有「dist 与 src 同步提交」的模式。
• HASH_NUM_RE 的注释（src/platform.ts:171-182）已随正则扩展同步更新，文档与代码保持一致。
• 但 parseExtraEnv 的 stderr 输出仍然发生在函数内，而退出码由调用方控制——「谁来负责失败通知」的责任分到了两处，未来如果调用方选择 return 0 而忽略 blockedKeys，会出现「日志说失败，进程却成功」的不一致窗口。这正是把 console.error 留在解析层、把 return 1 放在 main() 留下的隐含耦合。

散弹枪修改：本次修改只集中在 parseExtraEnv 周边（platform.ts、index.ts、编译产物、对应测试），是局部化的、目标清晰的修改，没有散弹式扩散。HASH_NUM_RE 的扩展也只动了正则与正则注释，外加新增的针对性单元测试。

阻塞项：无

建议项：

1. 考虑将 stderr 输出（console.error(...blocked...)）上移到 main() 中与 return 1 放在一起，使「失败通知」与「失败决策」在同一个调用栈帧内完成，避免调用方未来误用导致日志与退出码不一致。
2. parseExtraEnv 目前混合了纯解析、环境副作用、日志副作用，建议拆分为 parseExtraEnvSpec(): { allowed: [k,v][]; blocked: string[] }（纯函数）+ applyExtraEnv(spec)（副作用），便于单元测试与未来可能的 dry-run。
3. 长期看可考虑把 parseExtraEnv 从 platform.ts 迁出到 config.ts（如不存在则新建），让 platform.ts 专注于「GitHub/Gitea 评论抓取与 markdown 转义」这一抽象层。