[plugin/opencode] 7 issues in plugins/open-code-review/opencode/open-code-review.ts (incl. timeoutMinutes never wired to process timeout)

[waliean]

OpenCodeReview Version

open-code-review v1.8.6 (1b193db) windows/amd64 built at: 2026-08-03T06:41:26Z

Operating System

Windows

Installation Method

npm (global)

LLM Provider

Other OpenAI-compatible endpoint

Bug Description

使用 OpenCodeReview v1.8.6(OCR 托管模式,DeepSeek-V4-Flash,自定义 OpenAI 兼容提供商)审查它自己的 OpenCode 插件 plugins/open-code-review/opencode/open-code-review.ts 时发现以下问题。所有发现均由 OCR 生成,带行级定位。

高危

H1. timeoutMinutes 未接入进程级超时(open-code-review.ts:307-310)
timeoutMinutes 被透传为 OCR 的 --timeout(schema 描述明确为"Per-file OCR timeout in minutes",即每文件超时),但调用 runOcr 时未传 timeoutMs,将使用默认的 15 分钟整进程超时。当评审总耗时超过 15 分钟、或用户将 timeoutMinutes 配置为大于 15 时,整个评审进程会在用户配置的每文件超时生效前就被强制终止——用户配置实际无法生效。

中危

M1. await client.app.log 失败会阻断插件注册(open-code-review.ts:273-279)
插件初始化直接 await client.app.log,若日志服务调用失败,会拒绝整个插件初始化 Promise,导致 ocr_review/ocr_health 工具无法注册。日志属于非关键路径,不应阻断核心功能。

M2. context.abort.aborted 假设 abort 信号必然存在(open-code-review.ts:335-337)
runOcr 将 signal 定义为可选(signal?: AbortSignal)并全程使用可选链保护,但此处直接访问 context.abort.aborted,若运行环境未提供 abort 信号会抛 TypeError。

M3. resume 未与 commit/from/to 互斥校验(open-code-review.ts:71-73)
buildReviewArgs 仅拦截了 preview+resume、commit+range 的互斥,未拦截 resume 与 commit/from/to 同时传入的情况。用户或 agent 可能构造出 OCR CLI 不支持的参数组合,只能看到底层命令错误。

M4. 子进程未 detached,超时/取消后留下孤儿进程(open-code-review.ts:149-151)
超时、取消或输出超限时仅对直接子进程发送 SIGTERM/SIGKILL,spawn 未使用 detached 创建独立进程组。OCR 评审过程中会启动 git、LLM 等后代进程,父进程被杀后这些进程可能成为孤儿继续运行,造成资源泄漏、文件锁或后续操作异常。

低危

L1. 空输出抛出误导性的 "invalid JSON" 错误(open-code-review.ts:241-246)
非 preview 模式下直接对 stdout 执行 JSON.parse,若 OCR 成功退出但没有输出(如无变更场景),JSON.parse('') 会抛 "Unexpected end of JSON input",最终显示为误导性的 "OpenCodeReview returned invalid JSON",掩盖了"无变更"这一正常情况。

L2. ReviewInput.repo 为死代码(open-code-review.ts:309)
repo 在 ReviewInput 中已定义,但未在 reviewArgs schema 中暴露,且调用处以 repo: cwd 强制覆盖用户传入值。该参数实际不可达,用户输入会被静默忽略。

Steps to Reproduce

使用 OpenCodeReview v1.8.6(OCR 托管模式,DeepSeek-V4-Flash,自定义 OpenAI 兼容提供商)审查它自己的 OpenCode 插件 plugins/open-code-review/opencode/open-code-review.ts 时发现以下问题。所有发现均由 OCR 生成,带行级定位。

环境

• open-code-review v1.8.6(windows/amd64)
• 通过 ocr review --audience agent --format json 审查(OCR 托管模式)

Expected Behavior

本来只想试验一下插件的。就拿它扫了一下自己，结果真扫出了。难说是扫出的bug不准确还是确实存在这些问题

Logs / Error Output

Additional Context

No response

[lizhengfeng101]

感谢你拿 OCR 扫了一遍自己的插件代码，这个「自我审视」的实验很有趣。逐条核实了源码之后，客观分析如下：

---

H1. timeoutMinutes 未接入进程级超时 — ✅ 确实存在

这是一个真实的设计缺口。timeoutMinutes 作为 --timeout 传给了 OCR CLI 控制每文件超时，但 runOcr 的进程级超时始终是写死的 15 分钟。如果用户配了一个大值（比如 30 分钟），或者文件很多导致总耗时超过 15 分钟，进程会在用户预期之前被杀。不过实际严重程度可能没有"高危"那么夸张——多数评审在 15 分钟内能跑完，但确实应该修。

M1. await client.app.log 失败阻断注册 — ⚠️ 理论成立，实际影响极低

代码上确实是这样——如果 client.app.log 抛异常，插件初始化会失败。但 OpenCode 框架的内部日志接口在正常情况下不会抛异常（从测试里也能看出来就是个简单的异步函数）。加个 .catch(() => {}) 更稳妥，但这不太可能在生产中触发。

M2. context.abort.aborted 假设 abort 存在 — ❌ 不是 Bug（误报）

看了测试代码（toolContext 函数），OpenCode 插件框架的 tool 执行上下文必定提供 context.abort（一个 AbortSignal）。runOcr 把 signal 定义为可选只是因为它是个独立的工具函数，不耦合框架。直接访问 context.abort 是正确的，因为框架合约保证它存在。

M3. resume 未与 commit/from/to 互斥 — ⚠️ 改进建议，不算 Bug

确实没做这个校验。但即使同时传了，OCR CLI 自身会报错拒绝。加上校验能让错误信息更友好，但现有行为不会产生"静默错误结果"。算 nice-to-have。

M4. 子进程未 detached，可能留孤儿进程 — ⚠️ 理论正确，实践中不太会发生

OCR 是 Go 编译的二进制，收到 SIGTERM 后会通过自己的信号处理器清理子进程（git 等）。而且 detached: true 在 Windows 上行为不同，引入它反而会带来新的兼容性问题。这更像是一个设计权衡的讨论，不是当前代码的 Bug。

L1. 空输出抛误导性 "invalid JSON" — ⚠️ 理论存在，实际不会触发

OCR 在 --format json 模式下，即使无变更也会输出合法 JSON（如 {"findings":[]}）。只有 OCR 自身出 Bug 才会退出码为 0 但无输出。话虽如此，加一行空值检查确实更防御性编程，算是小改进。

L2. ReviewInput.repo 为死代码 — ❌ 不完全准确

repo 字段确实没暴露给用户（不在 schema 里），也总是被 cwd 覆盖。但它并非"死代码"——它参与了 buildReviewArgs 的逻辑，最终作为 --repo 传给 OCR CLI。设计意图是：强制使用工作区目录，不允许用户指定任意 repo 路径（安全考虑）。接口上可以精简掉这个字段让代码更干净，但它不会造成任何错误行为。

---

总结：7 条发现中，H1 是真实的设计缺陷值得修，M1/M3/L1 是合理的防御性编程改进建议但不算 Bug，M2 是误报，M4/L2 更多是设计风格讨论。

OCR 扫自己代码的实验效果不错——能发现真实问题（H1），但也存在一定的误报率，特别是在不了解框架合约（如 context.abort 必定存在）的情况下会过度报警。这也侧面说明给 OCR 提供更多上下文（比如框架的类型定义）能提升准确率。

H1 我会排进修复计划，其余的作为代码健壮性改进一并考虑。感谢反馈！

[Linxiushen]

I can work on the confirmed H1 timeout issue. I'll wire the configured per-file imeoutMinutes into the plugin's enclosing OCR process timeout as well, preserve the existing 15-minute default, and add focused regression coverage for both default and custom values. I'll keep the PR scoped to H1 rather than including the findings identified above as false positives or separate hardening ideas.

[waliean]

@gonzaleztheresa727 十分稳定，几乎没有位置漂移