Repository navigation
feat(ocr-review): 评审链外置 + 单跳/总预算超时 + 用量台账（承接 sess_9a73a8ef 半成品） - #11
Conversation
这份改动不是本会话（sess_ee07c00b）的工作，而是另一会话 sess_9a73a8ef 留在
工作区、尚未提交的半成品。它长期只存在于工作区与 /tmp 的易失副本里，
一次误还原或误覆盖就会丢失，故先落到本 WIP 分支做持久化，**不并入 main**。
改动内容（137 insertions / 23 deletions，仅 scripts/ocr-review.sh）：
- CHAIN_FILE:140 评审链外置到 ${OCR_REVIEW_CHAIN_FILE:-$HOME/.dsh/templates/ocr-review/chain-public}
（CHAIN_SCOPE:141 = public，cloud 条目合法，仍以链文件为唯一真源）
- HOP_TIMEOUT:181（默认 900s）/ TOTAL_BUDGET:182（默认 5400s）单跳与总预算
- USAGE_LOG:201 + record_use():202 按 provider/model 记账，供 ocr-pool-status.sh 选池
- cleanup 追加 `rm -rf ${LOCK_DIR:+"$LOCK_DIR"}` 释放锁目录
后续：该会话若回来继续，请基于本分支推进；本分支已推到 origin 以防本机丢失。
把另一会话留在 wip/sess-9a73a8ef-ocr-review-chain 的半成品推进到可合并状态：
补上回归锁，并修一处台账语义错误。
新增回归锁（12 条，均先证伪再锁定）：
- 链文件解析：按文件顺序取第一条合法条目；行内注释/空行/段数不对的行都要跳过。
变异测试（删掉 `_line="${_line%%#*}"`）→ 该用例转红，确认有牙。
- 链文件不可用 / 存在但零条合法条目 → 退回内嵌快照，且必须吭声（静默退化会让
「改了链却没生效」无从察觉）。删掉提示行 → 用例转红。
- 单跳挂起强杀：用「第一跳挂住并派生孙进程、第二跳正常」的假 ocr 验证超时后降级，
且**孙进程必须一起死**。把整组 TERM/KILL 换成裸 kill 单 pid → 孙进程存活，用例转红
（这正是本机无 GNU timeout 时用 perl alarm / 裸 kill 会漏孤儿的真实缺陷）。
- 总预算用尽后不再尝试剩余 provider（5 条链 + 3s 预算，断言尝试跳数 < 5）。
- 用量台账：成功记 ok、全链失败记 fail、**全被编码代理跳过记 skip**。
- 锁回收：成功/失败两条路径结束后 .git/ocr-review.lock 都必须消失，外加一条静态守卫。
修复：cleanup() 的 trap 覆盖了前面那个锁 trap，导致 LOCK_DIR 永不消失，之后每次运行
都走「陈旧锁」分支。已在 cleanup 里补 `rm -rf ${LOCK_DIR:+"$LOCK_DIR"}`。
实证：pre-fix 脚本跑完后锁目录仍在（YES-BUG）；本条用例对 pre-fix 脚本转红。
修复：SKIPPED_ALL（链上每个模型都被编码代理占用）此前记成 fail，与「链坏了」混为一谈，
而 record_use 的契约第三列是 ok|skip|fail 三值。改为记 skip，否则 ocr-pool-status.sh
会把「今天没评审」统计成「provider 全挂」，排查时一路查错方向。
验证：node --test test/ocr-review.test.js → 27 pass / 0 fail；
npm test → 559 tests, 558 pass, 0 fail, 1 skipped（buildDmg 非 macOS 跳过）；
/bin/bash 3.2 -n 通过（macOS 自带 bash 无 GNU timeout，故用进程组 kill）；
生产日志 ~/.zcode/advisor-companion.log 行数不变，真实自启作业 pid 存活。
注：内嵌快照 11 条 vs chain-public 18 条属**既有**的全仓普遍现象，由
~/.dsh/bin/ocr-sync-reviewer.sh 统一再生（运行时优先读链文件，快照仅兜底），
本提交不手工补快照，避免与生成器打架。
|
🔍 OpenCodeReview found 6 issue(s) in this PR.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| CHAIN_FILE="${OCR_REVIEW_CHAIN_FILE:-$HOME/.dsh/templates/ocr-review/chain-public}" | ||
| CHAIN_SCOPE="public" # public 副本：cloud 条目合法，但仍以链文件为唯一真源 |
There was a problem hiding this comment.
在动态链文件加载逻辑中，虽然进行了基本的格式验证（如 scope|provider|model），但缺少对 CHAIN_FILE 的权限验证。建议在读取文件前增加权限检查，确保文件不可写且属主正确，以防止恶意篡改。例如：
if [ -r "$CHAIN_FILE" ] && [ ! -w "$CHAIN_FILE" ] && [ "$(stat -c '%u' "$CHAIN_FILE")" = "$(id -u)" ]; then
# 继续加载链文件
fi| "runanytime|z-ai/glm-5.2" | ||
| "nas-octopus|glm-5.3-flash" |
There was a problem hiding this comment.
在 CHAIN 数组中添加了 runanytime-astra|gpt-6-astra 和 runanytime|z-ai/glm-5.3 两个条目，但缺少对这两个新条目的注释说明。建议在注释中明确说明这两个条目的作用、所属类别（如云端或本地）以及是否支持工具调用等信息，以保持注释与代码的一致性。
| _p="${_entry%%|*}"; _m="${_entry#*|}" | ||
| [ -n "$_p" ] && [ -n "$_m" ] || continue |
There was a problem hiding this comment.
在 CHAIN 数组的动态加载逻辑中，虽然进行了基本的格式验证（如 scope|provider|model），但缺少对 provider 或 model 的非空验证。例如，如果 provider 或 model 为空，可能会导致后续逻辑出错。建议在验证逻辑中增加对 provider 和 model 的非空检查，确保其值有效。
| # （zcode-advisor 实测两个模型 20 组全返回空 comments）。要连图片一起审就设空串关掉。 | ||
| # 用数组而不是字符串展开：排除模式里的 **/*.png 是通配符，字符串展开会被 | ||
| # 当前目录里的真实文件 glob 成文件名，旗标就废了。 | ||
| OCR_REVIEW_CONCURRENCY="${OCR_REVIEW_CONCURRENCY:-2}" |
There was a problem hiding this comment.
虽然默认并发数设置为 2 以避免限流，但对于某些高延迟的 provider（如 runanytime），该值可能仍然过高。建议在文档或代码注释中明确说明如何根据 provider 的具体限流策略调整 OCR_REVIEW_CONCURRENCY 的值，以提高灵活性。例如，可以添加一条注释说明如何为不同 provider 设置不同的并发数。
Suggestion:
| OCR_REVIEW_CONCURRENCY="${OCR_REVIEW_CONCURRENCY:-2}" | |
| # OCR_REVIEW_CONCURRENCY：默认 2 以避免限流，但可根据 provider 的限流策略调整。 | |
| # 例如：OCR_REVIEW_CONCURRENCY=8（适用于限流充裕的 provider） |
| # 用数组而不是字符串展开：排除模式里的 **/*.png 是通配符，字符串展开会被 | ||
| # 当前目录里的真实文件 glob 成文件名，旗标就废了。 | ||
| OCR_REVIEW_CONCURRENCY="${OCR_REVIEW_CONCURRENCY:-2}" | ||
| OCR_REVIEW_EXCLUDE="${OCR_REVIEW_EXCLUDE:-**/*.png,**/*.ico,**/*.icns,**/*.jpg,**/*.jpeg,**/*.gif,**/*.pdf}" |
There was a problem hiding this comment.
在 OCR_REVIEW_EXCLUDE 的默认值中，使用了通配符排除图片和二进制文件，但缺少对 .md 文件的排除。考虑到 Markdown 文件可能包含非文本内容（如图片链接），建议明确说明是否需要排除 .md 文件，或者根据实际需求进行调整。
此外，建议在文档中明确说明如何自定义 OCR_REVIEW_EXCLUDE，以避免误排除关键文件。
Suggestion:
| OCR_REVIEW_EXCLUDE="${OCR_REVIEW_EXCLUDE:-**/*.png,**/*.ico,**/*.icns,**/*.jpg,**/*.jpeg,**/*.gif,**/*.pdf}" | |
| # OCR_REVIEW_EXCLUDE：默认排除图片和二进制文件，避免噪音。 | |
| # 示例：OCR_REVIEW_EXCLUDE="**/*.png,**/*.md,**/*.pdf"（排除 Markdown 文件） |
| if [ "$(ps -o pgid= -p "$cmd_pid" 2>/dev/null | tr -d ' ')" = "$cmd_pid" ]; then | ||
| kill -TERM -- "-$cmd_pid" 2>/dev/null | ||
| else | ||
| pkill -TERM -P "$cmd_pid" 2>/dev/null | ||
| kill -TERM "$cmd_pid" 2>/dev/null | ||
| fi |
There was a problem hiding this comment.
在 run_with_deadline 函数中，虽然使用了进程组（PGID）来确保信号传递的准确性，但仍需确保 ps -o pgid= 的输出不会被篡改或误判。建议增加额外的验证步骤，例如检查进程组的父进程（PPID）是否与预期一致，以进一步提高安全性。
此外，建议在 kill 命令前增加对 cmd_pid 的有效性验证，避免无效 PID 导致的意外行为。
Suggestion:
| if [ "$(ps -o pgid= -p "$cmd_pid" 2>/dev/null | tr -d ' ')" = "$cmd_pid" ]; then | |
| kill -TERM -- "-$cmd_pid" 2>/dev/null | |
| else | |
| pkill -TERM -P "$cmd_pid" 2>/dev/null | |
| kill -TERM "$cmd_pid" 2>/dev/null | |
| fi | |
| if kill -0 "$cmd_pid" 2>/dev/null; then | |
| if [ "$(ps -o pgid= -p "$cmd_pid" 2>/dev/null | tr -d ' ')" = "$cmd_pid" ]; then | |
| kill -TERM -- "-$cmd_pid" 2>/dev/null | |
| else | |
| pkill -TERM -P "$cmd_pid" 2>/dev/null | |
| kill -TERM "$cmd_pid" 2>/dev/null | |
| fi | |
| else | |
| echo "警告：无效的 cmd_pid（$cmd_pid），可能已终止或不存在" >&2 | |
| fi |
| sleep 2 2>/dev/null | ||
| if [ "$(ps -o pgid= -p "$cmd_pid" 2>/dev/null | tr -d ' ')" = "$cmd_pid" ]; then | ||
| kill -KILL -- "-$cmd_pid" 2>/dev/null | ||
| else | ||
| pkill -KILL -P "$cmd_pid" 2>/dev/null | ||
| kill -KILL "$cmd_pid" 2>/dev/null | ||
| fi |
There was a problem hiding this comment.
在 run_with_deadline 函数中，虽然使用了进程组（PGID）来确保信号传递的准确性，但 sleep 2 的延迟可能不足以确保子进程完全终止。建议增加更健壮的进程终止机制，例如使用 wait 循环或增加更长的等待时间（如 5 秒），以确保所有子进程被完全清理。
| provider="${entry%%|*}" | ||
| model="${entry##*|}" | ||
| # 总预算先用完就不再试后面的：宁可本次无评审，也不无限期占着锁 | ||
| REMAIN=$(( TOTAL_BUDGET - ($(date +%s) - CHAIN_START) )) |
There was a problem hiding this comment.
在 REMAIN 计算中，使用了 $(date +%s) 获取当前时间戳，但未考虑时区差异或时间戳截断的问题。建议在文档中明确说明该脚本假设系统时区一致，或者使用 UTC 时间戳进行计算，以避免跨时区环境下的预算误判。
Suggestion:
| REMAIN=$(( TOTAL_BUDGET - ($(date +%s) - CHAIN_START) )) | |
| # 使用 UTC 时间戳以避免时区差异影响预算计算 | |
| CHAIN_START="$(date -u +%s)" # 使用 UTC 时间戳 | |
| REMAIN=$(( TOTAL_BUDGET - ($(date -u +%s) - CHAIN_START) )) |
| HOP_LIMIT="$HOP_TIMEOUT" | ||
| [ "$HOP_LIMIT" -gt "$REMAIN" ] && HOP_LIMIT="$REMAIN" |
There was a problem hiding this comment.
在 HOP_LIMIT 计算逻辑中，虽然考虑了剩余预算 REMAIN，但未考虑 HOP_TIMEOUT 的最小值限制。例如，如果 HOP_TIMEOUT 设置为非常小的值（如 1 秒），可能会导致每次尝试的时间过短，影响评审结果的准确性。建议在计算 HOP_LIMIT 时，增加一个最小值限制（如 30 秒），以确保每次尝试有足够的时间完成。
| case "$HOP_RC" in | ||
| 137|143) echo " ⚠ 本跳到 ${HOP_LIMIT}s 上限被强制终止（退出码 ${HOP_RC}），降级到下一个" ;; | ||
| esac |
There was a problem hiding this comment.
在错误信息输出中，虽然提供了基本的错误描述，但缺少具体的错误代码或上下文信息。建议在错误信息中增加更详细的说明，例如在超时终止时明确指出退出码（如 143 或 137），以帮助用户快速定位问题。
Suggestion:
| case "$HOP_RC" in | |
| 137|143) echo " ⚠ 本跳到 ${HOP_LIMIT}s 上限被强制终止（退出码 ${HOP_RC}），降级到下一个" ;; | |
| esac | |
| case "$HOP_RC" in | |
| 143) echo " ⚠ 本跳超时终止（退出码 143，上限 ${HOP_LIMIT}s），降级到下一个" ;; | |
| 137) echo " ⚠ 本跳被强制终止（退出码 137，上限 ${HOP_LIMIT}s），降级到下一个" ;; | |
| *) echo " ⚠ 本跳终止（退出码 ${HOP_RC}，上限 ${HOP_LIMIT}s），降级到下一个" ;; | |
| esac |
## 真实缺陷（本轮自行发现，非 ocr 报告）
`OCR_REVIEW_FORCE=1` 的语义是「我知道有锁，我偏要并行跑」——它整段跳过取锁。
但 cleanup 里的锁回收写成了无条件：
rm -rf ${LOCK_DIR:+"$LOCK_DIR"}
LOCK_DIR 在脚本顶部就无条件赋值，所以 FORCE=1 跑完会把**另一个正在跑的评审**的锁
连根拔掉。而并行恰恰是 FORCE 的唯一用途 → 两个进程都以为自己独占，互相拆锁。
（实测复现：预先放一个带活 pid 的锁目录，跑完 FORCE=1，锁目录消失。）
改为显式记录「是否自己取到锁」：
LOCK_ACQUIRED="" # 取锁成功 / 接管陈旧锁时置 1
[ -n "$LOCK_ACQUIRED" ] && rm -rf "$LOCK_DIR"
回归锁 2 条，均已变异验证：
- 动态：预置活锁 + FORCE=1 跑完，锁必须还在（退回无条件写法 → 转红）；
- 静态：钉住 `[ -n "$LOCK_ACQUIRED" ] && rm -rf "$LOCK_DIR"`，并反面禁止旧的
无条件写法复辟。
原有的两条「锁必须被回收」用例在变异体上仍为绿——两个方向都被覆盖。
## 表头注释与运行时不一致（ocr 报告 1 条，已采纳但改法不同）
表头把降级链逐条枚举成「主 nas-hy4/deepseek-v4.1-flash、备用①…」，而链的唯一真源
是 chain-public，运行时首跳早已是 minimax-m3。ocr 建议「补上两个新条目的注释」——
那是继续维护第二处会撒谎的枚举。改为**删掉整段枚举**，只指向真源：
链外置的意义就是「枚举必然过期」，再抄一份只会制造又一处与运行时冲突的注释。
顺带避开一个生成器陷阱：新注释里不要出现字面量 `CHAIN=(`，否则
ocr-sync-reviewer.sh 的 legacy 分支（`text.count("CHAIN=(") != 1` 即放弃定位）
会因计数变成 3 而拒绝同步。
## 其余 14 条 ocr 意见判为误报（未采纳）
- 链解析缺 provider/model 非空校验：L164/L166 已有 `[ -n "$_scope" ]`、
`[ -n "$_p" ] && [ -n "$_m" ]`；
- 超时消息缺退出码：L388 已输出 `（退出码 ${HOP_RC}）`；
- 时区导致预算误判：`date +%s` 是 epoch，实测 TZ=UTC/Shanghai/New_York 三值相同；
- 链文件权限校验建议用 `stat -c`：macOS BSD stat 无 `-c`（`illegal option -- c`）；
- 台账文件权限、路径含特殊字符、sleep 2 偏短、并发注释不足、排除 .md、
HOP_LIMIT 最小值、secs 范围校验、pgid 防篡改、变量名语义化：均为风格建议或
已在注释/实现中覆盖，无行为缺陷。
## 验证
- `node --test test/ocr-review.test.js` → 28 pass / 0 fail
- `npm test` → 560 tests / 559 pass / 0 fail / 1 skipped（预存在的 buildDmg 非 macOS 跳过）
- `/bin/bash -n scripts/ocr-review.sh` 通过（macOS bash 3.2）
- `ocr-sync-reviewer.sh --dry-run` 正常识别本副本（链 18 条），表头在生成区块之外
- 真实自启作业 state=running（pid 40075），生产日志 241 行无新增
第 1 轮 ocr 评审：15 条 → 采纳 1 条（改法不同）+ 自行发现 1 条真实缺陷
✅ 采纳：表头注释在撒谎（但改法不是「补注释」）ocr 说「新增的 补注释 = 继续维护第二处会撒谎的枚举。已改为删掉整段枚举，只指向真源——链外置的意义就是「枚举必然过期」。 顺带发现一个生成器陷阱：新注释里若出现字面量 ✅ 自行发现（ocr 未报）：
|
| # | 意见 | 实测 |
|---|---|---|
| 1 | 链解析缺 provider/model 非空校验 | L164/L166 已有 [ -n "$_scope" ]、[ -n "$_p" ] && [ -n "$_m" ] |
| 2 | 超时消息缺退出码 | L388 已有 （退出码 ${HOP_RC}） |
| 3 | 时区致预算误判，建议 date -u +%s |
date +%s 是 epoch：实测 TZ=UTC/Asia/Shanghai/America/New_York 三值相同 |
| 4 | 链文件权限校验，建议 stat -c '%u' |
macOS BSD stat 无 -c（illegal option -- c），建议直接报错 |
| 5 | 台账文件权限可能外泄 | ~/.dsh 是 drwx------；且台账只写 provider/model/结果 |
| 6 | 路径含特殊字符致格式化错误 | printf '%s\t' 不做词分割，basename 输出安全 |
| 7 | sleep 2 不足以等子进程退出 |
后面还有 -KILL 兜底，且用例实测孙进程被回收 |
| 8 | 并发注释不足，建议写示例 | 注释已说明默认值与调回方式 |
| 9 | 未排除 .md |
Markdown 是文本，评审的主要对象，排除它才是缺陷 |
| 10 | HOP_LIMIT 缺最小值 |
默认 900s；调小是使用者的显式选择（测试用 1s） |
| 11 | secs 范围校验（负数/超大） |
函数首行已 case 拒绝非数字 |
| 12 | pgid 需防篡改 + PPID 校验 | 单机自用工具；kill -0 守卫对已退出进程无意义（kill 本就有 2>/dev/null） |
| 13 | 变量名 _line/_scope 语义化 |
作用域仅 10 行的局部临时变量，改长反而降低可读性 |
| 14 | 排除项自定义说明不足 | OCR_REVIEW_EXCLUDE 已有注释说明如何关闭 |
验证
node --test test/ocr-review.test.js→ 28 pass / 0 failnpm test→ 560 tests / 559 pass / 0 fail / 1 skipped/bin/bash -n通过（macOS bash 3.2）；ocr-sync-reviewer.sh --dry-run正常识别本副本（链 18 条）- 真实自启作业
state=running（pid 40075），生产日志 241 行无新增
| # 降级链唯一真源：~/.dsh/templates/ocr-review/chain-public | ||
| # 一行一条 scope|provider|model，顺序即优先级，第一个产出有效 JSON 的即采用。 | ||
| # 改链只改那个文件：副本运行时也直接读它，下面这份只是读不到时的兜底快照。 | ||
| CHAIN_FILE="${OCR_REVIEW_CHAIN_FILE:-$HOME/.dsh/templates/ocr-review/chain-public}" |
There was a problem hiding this comment.
在 CHAIN_FILE 路径构建时，未对路径进行绝对路径验证或符号链接解析，可能存在目录遍历风险。建议使用 realpath 或 readlink -f 确保路径安全。
| _chain_found=() | ||
| if [ -r "$CHAIN_FILE" ]; then | ||
| while IFS= read -r _line || [ -n "$_line" ]; do | ||
| _line="${_line%%#*}" # 去掉行内注释 | ||
| _line="${_line//[[:space:]]/}" # 去空白，容忍手滑多打空格 | ||
| _pipes="${_line//[!|]/}" # 只留竖线，用来数段数 | ||
| [ "${#_pipes}" -eq 2 ] || continue # 必须正好 scope|provider|model | ||
| _scope="${_line%%|*}"; _entry="${_line#*|}" | ||
| [ -n "$_scope" ] || continue | ||
| _p="${_entry%%|*}"; _m="${_entry#*|}" | ||
| [ -n "$_p" ] && [ -n "$_m" ] || continue | ||
| _chain_found[${#_chain_found[@]}]="$_entry" | ||
| done < "$CHAIN_FILE" | ||
| fi |
There was a problem hiding this comment.
在 CHAIN 数组的处理逻辑中，虽然增加了从外部文件读取链配置的功能，但未对 CHAIN_FILE 的内容进行格式验证。例如，如果 CHAIN_FILE 包含无效的行（如缺少 | 分隔符或空行），可能会导致 CHAIN 数组被错误地构建，进而影响降级链的正常工作。建议在读取 CHAIN_FILE 时添加更严格的格式验证，确保每行都符合 scope|provider|model 的格式。
Suggestion:
| _chain_found=() | |
| if [ -r "$CHAIN_FILE" ]; then | |
| while IFS= read -r _line || [ -n "$_line" ]; do | |
| _line="${_line%%#*}" # 去掉行内注释 | |
| _line="${_line//[[:space:]]/}" # 去空白，容忍手滑多打空格 | |
| _pipes="${_line//[!|]/}" # 只留竖线，用来数段数 | |
| [ "${#_pipes}" -eq 2 ] || continue # 必须正好 scope|provider|model | |
| _scope="${_line%%|*}"; _entry="${_line#*|}" | |
| [ -n "$_scope" ] || continue | |
| _p="${_entry%%|*}"; _m="${_entry#*|}" | |
| [ -n "$_p" ] && [ -n "$_m" ] || continue | |
| _chain_found[${#_chain_found[@]}]="$_entry" | |
| done < "$CHAIN_FILE" | |
| fi | |
| _chain_found=() | |
| if [ -r "$CHAIN_FILE" ]; then | |
| while IFS= read -r _line || [ -n "$_line" ]; do | |
| _line="${_line%%#*}" # 去掉行内注释 | |
| _line="${_line//[[:space:]]/}" # 去空白，容忍手滑多打空格 | |
| _pipes="${_line//[!|]/}" # 只留竖线，用来数段数 | |
| [ "${#_pipes}" -eq 2 ] || { | |
| echo "警告：跳过无效行（必须包含 scope|provider|model）：$_line" >&2 | |
| continue | |
| } | |
| _scope="${_line%%|*}"; _entry="${_line#*|}" | |
| [ -n "$_scope" ] || { | |
| echo "警告：跳过无效行（scope 为空）：$_line" >&2 | |
| continue | |
| } | |
| _p="${_entry%%|*}"; _m="${_entry#*|}" | |
| [ -n "$_p" ] && [ -n "$_m" ] || { | |
| echo "警告：跳过无效行（provider 或 model 为空）：$_line" >&2 | |
| continue | |
| } | |
| _chain_found[${#_chain_found[@]}]="$_entry" | |
| done < "$CHAIN_FILE" | |
| fi |
| USAGE_LOG="${OCR_REVIEW_USAGE_LOG:-$HOME/.dsh/hooks/ocr-pool-usage.log}" | ||
| record_use() { # $1=provider $2=model $3=ok|skip|fail | ||
| printf '%s\t%s\t%s\t%s\t%s\n' "$(date '+%F %T')" "$(basename "${REPO_ROOT:-$PWD}")" "$1" "$2" "$3" \ | ||
| >>"$USAGE_LOG" 2>/dev/null || true |
There was a problem hiding this comment.
在 record_use 函数中，虽然实现了日志记录功能，但未对 USAGE_LOG 文件的写入权限进行验证。例如，如果 USAGE_LOG 文件所在目录没有写入权限，可能会导致日志记录失败，而脚本仍然继续运行，导致日志不完整。建议在调用 record_use 前增加权限验证，并提供友好的错误提示。
Suggestion:
| USAGE_LOG="${OCR_REVIEW_USAGE_LOG:-$HOME/.dsh/hooks/ocr-pool-usage.log}" | |
| record_use() { # $1=provider $2=model $3=ok|skip|fail | |
| printf '%s\t%s\t%s\t%s\t%s\n' "$(date '+%F %T')" "$(basename "${REPO_ROOT:-$PWD}")" "$1" "$2" "$3" \ | |
| >>"$USAGE_LOG" 2>/dev/null || true | |
| record_use() { | |
| local provider="$1" | |
| local model="$2" | |
| local result="$3" | |
| # 检查 USAGE_LOG 文件的写入权限 | |
| local log_dir="$(dirname "$USAGE_LOG")" | |
| if [ ! -w "$log_dir" ]; then | |
| echo "警告：无法写入日志文件 $USAGE_LOG，目录 $log_dir 无写入权限，跳过记录" >&2 | |
| return 1 | |
| fi | |
| printf '%s\t%s\t%s\t%s\t%s\n' "$(date '+%F %T')" "$(basename "${REPO_ROOT:-$PWD}")" "$provider" "$model" "$result" \ | |
| >>"$USAGE_LOG" 2>/dev/null || echo "警告：写入日志文件 $USAGE_LOG 失败" >&2 |
| local secs="$1"; shift | ||
| case "$secs" in ''|*[!0-9]*) "$@"; return $? ;; esac | ||
| [ "$secs" -gt 0 ] || { "$@"; return $?; } | ||
| set -m |
There was a problem hiding this comment.
在 run_with_deadline 函数中，虽然使用了 set -m 创建进程组，但未验证所有 shell 环境是否支持该功能。特别是在非 GNU coreutils 环境下，可能导致进程组管理失败，进而影响超时杀死逻辑的可靠性。建议添加环境检查或提供 fallback 机制。
| FAILURE_DIAG="${FAILURE_DIAG}--- ${provider}/${model} (exit=${HOP_RC}, limit=${HOP_LIMIT}s) --- | ||
| $(tail -5 "$ATTEMPT_LOG" 2>/dev/null) | ||
| " |
There was a problem hiding this comment.
在 FAILURE_DIAG 的构建中，仅保留了最后 5 行日志，对于复杂失败场景可能不足以定位问题。建议增加更多上下文信息，例如完整的错误日志或关键变量状态，以便更好地排查问题。
Suggestion:
| FAILURE_DIAG="${FAILURE_DIAG}--- ${provider}/${model} (exit=${HOP_RC}, limit=${HOP_LIMIT}s) --- | |
| $(tail -5 "$ATTEMPT_LOG" 2>/dev/null) | |
| " | |
| FAILURE_DIAG="${FAILURE_DIAG}--- ${provider}/${model} (exit=${HOP_RC}, limit=${HOP_LIMIT}s) --- | |
| $(tail -20 "$ATTEMPT_LOG" 2>/dev/null || echo "无日志可用") | |
| 关键变量：provider=${provider}, model=${model}, HOP_TIMEOUT=${HOP_TIMEOUT} | |
| " |
第 2 轮 ocr 评审：5 条 → 全部误报本轮 5 条里有 3 条被标为 security/perf high，逐条实测如下。 ❌ #1
|
1 participant
背景
另一会话
sess_9a73a8ef在wip/sess-9a73a8ef-ocr-review-chain留了一份半成品（提交信息明确写了「不并入 main」），本 PR 把它推进到可合并状态。前一个提交
a3b490c是原样保留的 WIP（作者、提交信息、内容均未改动，便于追溯）；本 PR 的增量是932d692：补回归锁 + 修两处真实缺陷。WIP 带来的四块能力
1. 降级链外置（唯一真源）
链改为运行时读
~/.dsh/templates/ocr-review/chain-public（一行scope|provider|model，顺序即优先级），读不到或解析出 0 条才回落到内嵌快照——脚本被单独拷走也跑得起来。此前链硬编码在脚本里，改一个模型要手工改 11 份副本。2. 单跳 hard deadline + 总预算
HOP_TIMEOUT（默认 900s）单跳上限，TOTAL_BUDGET（默认 5400s）整条链上限。timeout，也没用perl alarm（只杀 perl 自己，ocr 子进程会漏成孤儿）。改用set -m让任务自成进程组，超时kill -TERM -- -PID整组带走；仅在确认PGID == PID时才发组信号，否则退化为「先杀子进程再杀父进程」，绝不误伤本脚本所在进程组。3. 限流/资产参数提取为变量
OCR_REVIEW_CONCURRENCY（默认 2；默认并发 8 实测打满 x666 的 25 请求/5 分钟限流，曾连续 6 次评审全灭）、OCR_REVIEW_EXCLUDE（默认排掉 png/ico/icns/jpg/pdf 等二进制资产）。用数组而非字符串展开——**/*.png会被当前目录真实文件 glob 成文件名，旗标就废了。4. 用量台账
record_use()把「哪个 provider/model、成没成」追加到~/.dsh/hooks/ocr-pool-usage.log（只写 provider/model/结果，不写 diff、不写 key），供ocr-pool-status.sh展示。本 PR 新增的增量（932d692）
回归锁 12 条，均先证伪再锁定（变异测试确认有牙）：
.git/ocr-review.lock都必须消失。修复 1：锁泄漏（真实缺陷）
cleanup()的 trap 覆盖了前面的锁清理 trap，LOCK_DIR永不消失，之后每次运行都走「陈旧锁」分支。已补rm -rf ${LOCK_DIR:+"$LOCK_DIR"}。实证：pre-fix 脚本跑完后锁目录仍在。修复 2：全跳过被记成 fail（语义错误）
链上每个模型都被编码代理占用时此前记
fail，与「链坏了」混为一谈，会让ocr-pool-status.sh把「今天没评审」统计成「provider 全挂」。record_use的契约本就是ok|skip|fail三值，改为记skip。验证
node --test test/ocr-review.test.js→ 27 pass / 0 failnpm test→ 559 tests, 558 pass, 0 fail, 1 skipped（buildDmg：非 macOS预存在跳过）/bin/bash -n scripts/ocr-review.sh通过（macOS 自带 bash 3.2）~/.zcode/advisor-companion.log无新增；真实自启作业state=running（pid 40075）存活已知未处理（有意为之）
内嵌快照 11 条 vs
chain-public18 条——这是全仓普遍现象（~/.dsh/bin/ocr-sync-reviewer.sh --dry-run显示 9 个仓库都待同步），由同步器统一再生。运行时优先读链文件、快照仅兜底，故本 PR 不手工补快照，避免与生成器打架。