From a3b490c97d76c6b66939833286c84781e66dac9e Mon Sep 17 00:00:00 2001 From: Seller-1990 Date: Tue, 6 Oct 2026 11:58:12 +0800 Subject: [PATCH 1/3] =?UTF-8?q?WIP(ocr-review):=20=E8=AF=84=E5=AE=A1?= =?UTF-8?q?=E9=93=BE=E5=A4=96=E7=BD=AE=20+=20=E7=94=A8=E9=87=8F=E8=AE=B0?= =?UTF-8?q?=E8=B4=A6=20+=20=E9=A2=84=E7=AE=97=E6=8E=A7=E5=88=B6=EF=BC=88?= =?UTF-8?q?=E5=8F=A6=E4=B8=80=E4=BC=9A=E8=AF=9D=20sess=5F9a73a8ef=20?= =?UTF-8?q?=E6=9C=AA=E5=AE=8C=E6=88=90=EF=BC=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 这份改动不是本会话（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 以防本机丢失。 --- scripts/ocr-review.sh | 160 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 137 insertions(+), 23 deletions(-) diff --git a/scripts/ocr-review.sh b/scripts/ocr-review.sh index 3fae5b2..365b82c 100755 --- a/scripts/ocr-review.sh +++ b/scripts/ocr-review.sh @@ -7,18 +7,19 @@ # - 也**不定时跑**：只在每个 PR 合并前调用一次。 # # 模型降级链（provider|model，与 ~/.opencodereview/config.json 的 custom_providers 对应）： -# ⚠️ 本脚本含云端 provider，**只允许公开仓使用**（云端会把代码发往第三方）； -# 私有仓用 ocr-review.sh.private 变体（NAS-only，见 ~/.dsh/templates/ocr-review/）。 -# 前四棒全 0 倍率免费（2026-10-04 成本优先，避开 kimi-k3 等贵模型）： +# 【2026-10-04】隐私铁律暂缓（用户拍板）：私有仓也可用本链（私有副本用 ocr-review.sh.private +# 变体，链走 chain-private，同样允许 cloud 条目）。 # 主 nas-hy4 -> deepseek-v4.1-flash （8787 免费，工具调用已实测） -# 备用① nas-octopus -> glm-5.3-flash （8088 PM-API 免费分组） -# 备用② nas-hy4 -> hy3 （8787 免费） -# 备用③ nas-hy4 -> hy4-preview-f （8787 免费） -# 备用④ x666 -> ministral-14b-latest （云端 0 倍率，工具调用已实测） -# 备用⑤ lucky-gem -> gemini-3.6-flash （云端，主人提供 key，工具调用已实测） +# 备用① runanytime -> z-ai/glm-5.2 （随时跑路中转，本机中继 127.0.0.1:18789，工具调用已实测） +# 备用② nas-octopus -> glm-5.3-flash （8088 PM-API 免费分组） +# 备用③ nas-hy4 -> hy3 （8787 免费） +# 备用④ nas-hy4 -> hy4-preview-f （8787 免费） +# 备用⑤ x666 -> ministral-14b-latest （云端 0 倍率，工具调用已实测；该站唯一可用，2026-10-04 全站复测） # 备用⑥ lucky -> stealth/space-bunny-alpha （云端，工具调用已实测） # 备用⑦ lucky -> step-5-preview （云端，工具调用已实测） # 备用⑧ daigua -> gpt-6-sol （云端，顾问生产验证过工具调用） +# （lucky-gem/gemini-3.6-flash 已出链：2026-10-04 实测 503 无通道） +# ⚠️ 同站 kimi-k3 不支持工具调用（2026-10-04 三次探针无 tool_calls）→ 不能当 ocr 模型，勿加回。 # 用法： # ./scripts/ocr-review.sh # 默认基准 origin/dev（不存在则回退 main） # ./scripts/ocr-review.sh v1.8.8 # 对比 tag / 分支 @@ -132,18 +133,76 @@ fi AGENT_MODEL="$(tr -d '[:space:]' < "$AGENT_MODEL_FILE" 2>/dev/null || true)" [ -n "$AGENT_MODEL" ] && echo "（编码代理当前模型：${AGENT_MODEL}，评审将优先使用其他 provider）" -# 降级链：逐个尝试，第一个**产出有效 JSON** 的即采用 +# === 降级链区块开始（由 ~/.dsh/bin/ocr-sync-reviewer.sh 从 chain-public 生成，勿手改）=== +# 降级链唯一真源：~/.dsh/templates/ocr-review/chain-public +# 一行一条 scope|provider|model，顺序即优先级，第一个产出有效 JSON 的即采用。 +# 改链只改那个文件：副本运行时也直接读它，下面这份只是读不到时的兜底快照。 +CHAIN_FILE="${OCR_REVIEW_CHAIN_FILE:-$HOME/.dsh/templates/ocr-review/chain-public}" +CHAIN_SCOPE="public" # public 副本：cloud 条目合法，但仍以链文件为唯一真源 CHAIN=( "nas-hy4|deepseek-v4.1-flash" + "runanytime|z-ai/glm-5.2" "nas-octopus|glm-5.3-flash" "nas-hy4|hy3" "nas-hy4|hy4-preview-f" "x666|ministral-14b-latest" - "lucky-gem|gemini-3.6-flash" + "runanytime-astra|gpt-6-astra" + "runanytime|z-ai/glm-5.3" "lucky|stealth/space-bunny-alpha" "lucky|step-5-preview" "daigua|gpt-6-sol" ) +# 运行时优先读链文件：读不到 / 解析出 0 条 → 保留上面的快照（脚本被单独拷走也跑得起来）。 +_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 +if [ "${#_chain_found[@]}" -gt 0 ]; then + CHAIN=("${_chain_found[@]}") +else + echo "（链文件 ${CHAIN_FILE} 不可用，用内嵌快照 ${#CHAIN[@]} 条）" >&2 +fi +# === 降级链区块结束 === + +# 时间预算：防"建连后永久挂起"把整条链卡死（本机无 GNU timeout，见 run_with_deadline）。 +# HOP_TIMEOUT 单跳上限。完整评审实测 5–15 分钟，900s 覆盖正常耗时，只掐挂起。 +# TOTAL_BUDGET 整条链最长耗时。公开链条目多，按 6 跳封顶——再长会把 +# .git/ocr-review.lock 占用到之后每次推送都被误判"已有评审在进行"。 +HOP_TIMEOUT="${OCR_REVIEW_HOP_TIMEOUT:-900}" +TOTAL_BUDGET="${OCR_REVIEW_TOTAL_BUDGET:-5400}" +CHAIN_START="$(date +%s)" + +# 上游限流与资产噪音的稳定参数（都是实测踩出来的教训，不是默认值拍脑袋）： +# --concurrency：ocr 默认并发 8 个子任务，实测会打满 x666 的「25 请求/5 分钟」限流（429）， +# 曾连续 6 次评审全灭。降并发换稳；限流充裕的 provider 用 OCR_REVIEW_CONCURRENCY=8 调回。 +# --exclude：二进制图片资产进 diff 只会塞爆每个分组的上下文，对文本评审是纯噪音 +# （zcode-advisor 实测两个模型 20 组全返回空 comments）。要连图片一起审就设空串关掉。 +# 用数组而不是字符串展开：排除模式里的 **/*.png 是通配符，字符串展开会被 +# 当前目录里的真实文件 glob 成文件名，旗标就废了。 +OCR_REVIEW_CONCURRENCY="${OCR_REVIEW_CONCURRENCY:-2}" +OCR_REVIEW_EXCLUDE="${OCR_REVIEW_EXCLUDE:-**/*.png,**/*.ico,**/*.icns,**/*.jpg,**/*.jpeg,**/*.gif,**/*.pdf}" +OCR_EXTRA_ARGS=() +OCR_EXTRA_ARGS+=(--concurrency "$OCR_REVIEW_CONCURRENCY") +[ -n "$OCR_REVIEW_EXCLUDE" ] && OCR_EXTRA_ARGS+=(--exclude "$OCR_REVIEW_EXCLUDE") + +# 实际使用台账：每次评审把"用了哪个 provider/model、成没成"追加到一份公共日志， +# 供 ~/.dsh/hooks/ocr-pool-status.sh 展示最近几跳（否则没人知道当前到底在用谁）。 +# 只写 provider/model 名与结果，不写 diff 内容、不写 key。 +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 +} # 模板必须以 XXXXXX 结尾：GNU mktemp 把 -t 当 --tmpdir 并要求该后缀（BSD/macOS 两者都接受）。 TMPDIR_OCR="${TMPDIR:-/tmp}" @@ -152,6 +211,8 @@ OUT_JSON="$(mktemp "${TMPDIR_OCR%/}/ocr-review-XXXXXX" 2>/dev/null)" || { } [ -n "$OUT_JSON" ] || { echo "错误：mktemp 未返回路径" >&2; exit 1; } ATTEMPT_LOG="" +# 注意：这个 trap 会覆盖前面的锁清理 trap，所以 rm -rf "$LOCK_DIR" 必须在这里再做一次， +# 否则锁目录永不消失，之后每次运行都误报"陈旧锁"（虽然能接管，但看着像坏了）。 # OCR_REVIEW_RAW_OUT（可选，由调用方提供，例如 pre-push hook）：把 ocr 的**原始 JSON** # 另存一份。它是权威产物（下面的渲染表由它生成），而 OUT_JSON 只是 mktemp 临时文件、 # 退出时被本函数删掉 —— 此前评审成功后原始 JSON 一并消失，事后无法复核或重渲染。 @@ -201,7 +262,7 @@ cleanup() { # （dirname 对 "/foo.json" 返回 "/"、对 "foo.json" 返回 "."，**永不为空**，故无需兜底。） fi fi - rm -f "$OUT_JSON" ${ATTEMPT_LOG:+"$ATTEMPT_LOG"} + rm -f "$OUT_JSON" ${ATTEMPT_LOG:+"$ATTEMPT_LOG"}; rm -rf ${LOCK_DIR:+"$LOCK_DIR"} } trap cleanup EXIT INT TERM @@ -242,6 +303,50 @@ if not isinstance(data.get("comments"), list): PYCHECK } +# hard deadline：给一条命令套"最多 secs 秒"的上限，超时整组终止（含孙进程）。 +# +# 为什么不用 `timeout`：本机是 macOS + bash 3.2，没有 GNU coreutils 的 timeout， +# 也没有 gtimeout（防止某天被装进来）。 +# 为什么不用 `perl -e 'alarm N; exec @ARGV'`：alarm 只杀 perl 自己变出的那个进程， +# ocr 派生的子进程会漏成孤儿。这里改用"自成进程组 + 整组 signale"： +# set -m 让后台任务成为组长（PGID == PID），超时时 kill -TERM -- -PID 全组带走。 +# 安全细节：只有在确认目标 PGID 等于自身 PID 时才发组信号，否则退化成 +# "先杀子进程、再杀父进程"，绝不可能误伤本脚本所在的进程组。 +# 退出码：快命令原样透传；被杀时 143(TERM)/137(KILL)，调用方据此判定超时。 +run_with_deadline() { + local secs="$1"; shift + case "$secs" in ''|*[!0-9]*) "$@"; return $? ;; esac + [ "$secs" -gt 0 ] || { "$@"; return $?; } + set -m + "$@" & + local cmd_pid=$! + set +m + ( + sleep "$secs" 2>/dev/null + 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 + 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 + ) & + local watcher_pid=$! + # 下面三行的 || rc=$? / || true 是给 set -e 型调用方兜底：超时后这一跳返回 143， + # 裸 wait 在 set -euo pipefail 的脚本里会直接把整个评审带走，链就不再降级了（实测踩过）。 + local rc=0 + wait "$cmd_pid" 2>/dev/null || rc=$? + kill "$watcher_pid" 2>/dev/null || true + wait "$watcher_pid" 2>/dev/null || true + return $rc +} + echo "评审范围：$FROM .. $TO" CHOSEN="" SKIPPED=0 @@ -250,33 +355,41 @@ FAILURE_DIAG="" for entry in "${CHAIN[@]}"; do provider="${entry%%|*}" model="${entry##*|}" + # 总预算先用完就不再试后面的：宁可本次无评审，也不无限期占着锁 + REMAIN=$(( TOTAL_BUDGET - ($(date +%s) - CHAIN_START) )) + if [ "$REMAIN" -le 0 ]; then + echo " 总预算 ${TOTAL_BUDGET}s 已用尽，剩余 provider 不再尝试" + break + fi if [ -n "$AGENT_MODEL" ] && [ "$model" = "$AGENT_MODEL" ]; then echo " 跳过 ${provider}（$model 正被编码代理占用）" SKIPPED=$((SKIPPED + 1)) [ "$SKIPPED" -eq "${#CHAIN[@]}" ] && SKIPPED_ALL=1 continue fi + HOP_LIMIT="$HOP_TIMEOUT" + [ "$HOP_LIMIT" -gt "$REMAIN" ] && HOP_LIMIT="$REMAIN" # 每次尝试前清空，防止失败后残留上次结果被误判为成功 : > "$OUT_JSON" ATTEMPT_LOG="$(mktemp "${TMPDIR_OCR%/}/ocr-attempt-XXXXXX" 2>/dev/null || true)" - echo " 尝试 provider=$provider model=$model …" - # --concurrency 2：ocr 默认并发 8 个子任务（每组还有多轮工具调用），实测会打满 - # x666 的「25 请求/5 分钟」限流（429）——连续 6 次评审全灭的根因。降并发换稳。 - # 可用 OCR_REVIEW_CONCURRENCY 覆盖；限流充裕的 provider 可设回 8。 - # --exclude 图片：assets/icon.png 是 ~1MB 二进制，进 diff 会塞爆每个分组的上下文， - # 实测两个模型全部返回空 comments（20 组全灭）——二进制资产对文本评审只有噪音。 - if ocr review --from "$FROM" --to "$TO" --format json --output "$OUT_JSON" \ - --concurrency "${OCR_REVIEW_CONCURRENCY:-2}" \ - --exclude '**/*.png,**/*.ico,**/*.icns,**/*.jpg,**/*.jpeg,**/*.gif,**/*.pdf' \ - --provider "$provider" --model "$model" >"${ATTEMPT_LOG:-/dev/null}" 2>&1 && valid_output; then + echo " 尝试 provider=$provider model=$model （上限 ${HOP_LIMIT}s）…" + HOP_RC=0 + run_with_deadline "$HOP_LIMIT" ocr review --from "$FROM" --to "$TO" --format json --output "$OUT_JSON" \ + ${OCR_EXTRA_ARGS[@]+"${OCR_EXTRA_ARGS[@]}"} \ + --provider "$provider" --model "$model" >"${ATTEMPT_LOG:-/dev/null}" 2>&1 || HOP_RC=$? + if [ "$HOP_RC" -eq 0 ] && valid_output; then echo " 成功：$provider / $model" CHOSEN="$provider/$model" + record_use "$provider" "$model" ok [ -n "$ATTEMPT_LOG" ] && rm -f "$ATTEMPT_LOG" break fi + case "$HOP_RC" in + 137|143) echo " ⚠ 本跳到 ${HOP_LIMIT}s 上限被强制终止（退出码 ${HOP_RC}），降级到下一个" ;; + esac # 保留该次失败输出尾部，供全失败时排查（此前一律丢进 /dev/null） if [ -n "$ATTEMPT_LOG" ]; then - FAILURE_DIAG="${FAILURE_DIAG}--- ${provider}/${model} --- + FAILURE_DIAG="${FAILURE_DIAG}--- ${provider}/${model} (exit=${HOP_RC}, limit=${HOP_LIMIT}s) --- $(tail -5 "$ATTEMPT_LOG" 2>/dev/null) " rm -f "$ATTEMPT_LOG" @@ -285,11 +398,12 @@ $(tail -5 "$ATTEMPT_LOG" 2>/dev/null) done if [ -z "$CHOSEN" ]; then + record_use "-" "-" fail if [ "$SKIPPED_ALL" = "1" ]; then echo "错误：所有 provider 都因与编码代理同模型被跳过（agent-model=${AGENT_MODEL}）。" >&2 echo " 可临时指定：ocr review --provider

--model " >&2 else - echo "错误：三级 provider 全部失败，未产生有效评审结果。" >&2 + echo "错误：${TOTAL_BUDGET}s 预算内没有一跳产出有效评审结果。" >&2 fi if [ -n "$FAILURE_DIAG" ]; then echo "各 provider 输出尾部：" >&2 From 932d692862f89e8b1d1206d6a688569b0d3eff7f Mon Sep 17 00:00:00 2001 From: Seller-1990 Date: Tue, 6 Oct 2026 16:20:49 +0800 Subject: [PATCH 2/3] =?UTF-8?q?test(ocr-review):=20=E9=92=89=E4=BD=8F?= =?UTF-8?q?=E9=93=BE=E5=A4=96=E7=BD=AE/=E6=97=B6=E9=97=B4=E9=A2=84?= =?UTF-8?q?=E7=AE=97/=E7=94=A8=E9=87=8F=E5=8F=B0=E8=B4=A6/=E9=94=81?= =?UTF-8?q?=E5=9B=9E=E6=94=B6=EF=BC=8C=E5=B9=B6=E4=BF=AE=E5=85=A8=E8=B7=B3?= =?UTF-8?q?=E8=BF=87=E8=AE=B0=E6=88=90=20fail?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 把另一会话留在 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 统一再生（运行时优先读链文件，快照仅兜底）， 本提交不手工补快照，避免与生成器打架。 --- scripts/ocr-review.sh | 5 +- test/ocr-review.test.js | 235 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 239 insertions(+), 1 deletion(-) diff --git a/scripts/ocr-review.sh b/scripts/ocr-review.sh index 365b82c..1cc3f99 100755 --- a/scripts/ocr-review.sh +++ b/scripts/ocr-review.sh @@ -398,11 +398,14 @@ $(tail -5 "$ATTEMPT_LOG" 2>/dev/null) done if [ -z "$CHOSEN" ]; then - record_use "-" "-" fail if [ "$SKIPPED_ALL" = "1" ]; then + # 与「链坏了」区分开：这不是 provider 失败，是编码代理占用了链上每一个模型。 + # 记 skip 而不是 fail，否则台账会把「今天没评审」统计成「provider 全挂」。 + record_use "-" "-" skip echo "错误：所有 provider 都因与编码代理同模型被跳过（agent-model=${AGENT_MODEL}）。" >&2 echo " 可临时指定：ocr review --provider

--model " >&2 else + record_use "-" "-" fail echo "错误：${TOTAL_BUDGET}s 预算内没有一跳产出有效评审结果。" >&2 fi if [ -n "$FAILURE_DIAG" ]; then diff --git a/test/ocr-review.test.js b/test/ocr-review.test.js index 7156bab..449651c 100644 --- a/test/ocr-review.test.js +++ b/test/ocr-review.test.js @@ -36,6 +36,32 @@ const skipUnlessHardened = SUPPORTS_RAW_OUT_HARDENED ? {} : { skip: 'scripts/ocr-review.sh 的 RAW_OUT 尚未加固（原子发布/目录拒绝）' }; +// 链外置 / 时间预算 / 用量台账 / 锁清理 是另一处改动引入的能力，与 RAW_OUT 同文件但独立。 +// 逐项探测、缺哪项跳哪项，免得「同文件里一个特性已合入、另一个还没合」时整套测试被染红。 +const SUPPORTS_CHAIN_FILE = REVIEWER_SRC_TEXT.includes('OCR_REVIEW_CHAIN_FILE'); +const SUPPORTS_DEADLINE = REVIEWER_SRC_TEXT.includes('run_with_deadline'); +const SUPPORTS_BUDGET = REVIEWER_SRC_TEXT.includes('OCR_REVIEW_TOTAL_BUDGET'); +const SUPPORTS_USAGE_LOG = REVIEWER_SRC_TEXT.includes('OCR_REVIEW_USAGE_LOG'); +// 锁清理：cleanup() 的 trap 会覆盖前面那个锁 trap，故必须在 cleanup 里再删一次。 +// 钉住的是「cleanup 里必须出现 rm -rf "$LOCK_DIR"」这一实现事实。 +const SUPPORTS_LOCK_CLEANUP = /rm -rf \$\{LOCK_DIR:\+"\$LOCK_DIR"\}/.test(REVIEWER_SRC_TEXT); + +const skipUnlessChainFile = SUPPORTS_CHAIN_FILE + ? {} + : { skip: 'scripts/ocr-review.sh 尚不支持 OCR_REVIEW_CHAIN_FILE' }; +const skipUnlessDeadline = SUPPORTS_DEADLINE + ? {} + : { skip: 'scripts/ocr-review.sh 尚无 run_with_deadline（时间预算）' }; +const skipUnlessBudget = SUPPORTS_BUDGET + ? {} + : { skip: 'scripts/ocr-review.sh 尚不支持 OCR_REVIEW_TOTAL_BUDGET' }; +const skipUnlessUsageLog = SUPPORTS_USAGE_LOG + ? {} + : { skip: 'scripts/ocr-review.sh 尚不支持 OCR_REVIEW_USAGE_LOG' }; +const skipUnlessLockCleanup = SUPPORTS_LOCK_CLEANUP + ? {} + : { skip: 'scripts/ocr-review.sh 的 cleanup 尚未回收 LOCK_DIR' }; + const COMPLETE_JSON = { status: 'complete', comments: [{ path: 'x.js', severity: 'high', content: '示例意见' }], @@ -396,3 +422,212 @@ test('ocr-review.sh：发布出的原始 JSON 权限不得宽于 0600（评审 const mode = fs.statSync(raw).mode & 0o777; assert.strictEqual(mode & 0o077, 0, `不得对组/其他用户开放，实际权限 ${mode.toString(8)}`); }); + +// ── 降级链外置 / 时间预算 / 用量台账 / 锁回收 ───────────────────────────────── +// 这四项是「评审链外置」那处改动引入的。教训背景： +// ① 改链要改脚本正文 → 每次调链都是一次代码改动 + 一轮评审；真源应外置。 +// ② 本机无 GNU timeout，一次建连挂起就能把整条链卡死，且锁被长期占用， +// 之后每次推送都被误判「已有评审在进行」。 +// ③ 没人知道当前到底在用哪个 provider（配额打满后一路降级到谁，全靠猜）。 + +// 链文件的解析规则：一行一条 scope|provider|model，行内注释/空行/段数不对的行都要跳过， +// 且必须**按文件顺序**取第一条可用的（顺序即优先级）。 +test('ocr-review.sh：链文件按顺序解析，跳过注释/空行/段数不对的行', skipUnlessChainFile, () => { + const sb = mkSandbox(); + // 故意把干扰项放在最前面：若解析器不过滤，第一跳就会是 bad|extra 或空模型。 + fs.writeFileSync( + sb.chain, + [ + '# scope|provider|model', + '', + ' ', + 'public|fake|bad|extra', // 3 个竖线：段数不对 + 'public||empty-model', // 模型为空 + 'public|fake|m1 # 行内注释必须被剥掉，否则 model 会带尾巴', + 'public|fake|m2' + ].join('\n') + '\n' + ); + + const out = runReviewer(sb, {}); + // model 必须恰好是 m1（行内注释被剥掉），而不是 "m1#行内注释必须被剥掉..." + assert.match(out, /成功：fake \/ m1\b/, `应取第一条合法条目，实际输出：\n${out}`); + assert.doesNotMatch(out, /成功：fake \/ m2/, '第一条合法条目之后的不应被用到'); +}); + +// 链文件读不到（或解析出 0 条）时必须退回内嵌快照，脚本被单独拷走也要能跑。 +test('ocr-review.sh：链文件不可用时退回内嵌快照并吭声', skipUnlessChainFile, () => { + const sb = mkSandbox(); + const missing = path.join(sb.root, 'no-such-chain'); + const r = runReviewerSync(sb, { OCR_REVIEW_CHAIN_FILE: missing }); + + assert.strictEqual(r.code, 0, '退回快照后评审仍应成功'); + assert.match(r.stderr, /链文件.*不可用/, '静默退回会让「改了链却没生效」无从察觉'); + assert.match(r.stderr, /内嵌快照 \d+ 条/, '应说明快照里有多少条，便于判断是否退化'); + assert.match(r.stdout, COMPLETED_RE, '快照里的第一跳应能完成评审'); +}); + +test('ocr-review.sh：链文件存在但零条合法条目时同样退回快照', skipUnlessChainFile, () => { + const sb = mkSandbox(); + fs.writeFileSync(sb.chain, '# 全是注释\nbadline\npublic||\n'); + const r = runReviewerSync(sb, {}); + assert.strictEqual(r.code, 0); + assert.match(r.stderr, /链文件.*不可用/, '解析出 0 条等同于不可用，必须退回快照'); + assert.match(r.stdout, COMPLETED_RE); +}); + +// 单跳超时：本机 macOS + bash 3.2 没有 GNU timeout，run_with_deadline 用「自成进程组 + +// 整组 TERM」实现。这里锁住两个行为：① 挂起的一跳会被掐掉并降级；② **孙进程**也必须死 +// （用 perl alarm 或裸 kill 只杀直接子进程，ocr 派生的子进程会漏成孤儿）。 +test('ocr-review.sh：单跳挂起会被强杀并降级（退出码 137/143 透传）', skipUnlessDeadline, () => { + const sb = mkSandbox(); + const childPidFile = path.join(sb.root, 'grandchild.pid'); + const state = path.join(sb.root, 'state'); + fs.mkdirSync(state, { recursive: true }); + // 第 1 跳：派生一个「孙进程」并记下 pid，然后自己也挂住 —— 整组被杀后孙进程必须一起死。 + // 第 2 跳：正常返回完整 JSON，用来验证「超时后确实降级了」。 + // 用计数文件区分两跳，而不是让假 ocr 一律挂起（那样第二跳也会挂，就测不到降级）。 + writeFakeOcr( + sb, + `#!/usr/bin/env bash +out="" +while [ $# -gt 0 ]; do case "$1" in --output) out="$2"; shift 2;; *) shift;; esac; done +n=0 +if [ -f "${state}/count" ]; then n=$(cat "${state}/count"); fi +n=$((n + 1)) +printf '%s' "$n" > "${state}/count" +if [ "$n" -eq 1 ]; then + sleep 300 & + echo $! > "${childPidFile}" + sleep 300 +fi +printf '%s\\n' '${JSON.stringify(COMPLETE_JSON)}' > "$out" +` + ); + fs.writeFileSync(sb.chain, 'public|fake|hang\npublic|fake|m1\n'); + + const r = runReviewerSync(sb, { OCR_REVIEW_HOP_TIMEOUT: '2' }); + assert.strictEqual(r.code, 0, '第一跳超时后应降级到第二跳并成功'); + // 超时提示走 stdout（与「尝试 provider=…」同一路），且必须带上真实退出码， + // 否则看起来和「普通失败」一模一样，排查时会误判成 provider 坏了。 + assert.match(r.stdout, /上限被强制终止（退出码 (137|143)）/, '超时必须明说并带上退出码'); + assert.match(r.stdout, /成功：fake \/ m1/, '应降级到下一跳'); + + // 孙进程必须已被回收（组信号生效）。给它一点时间完成 TERM→KILL。 + const gcPid = fs.readFileSync(childPidFile, 'utf8').trim(); + const alive = () => { + try { + process.kill(Number(gcPid), 0); + return true; + } catch (_) { + return false; + } + }; + const deadline = Date.now() + 5000; + while (alive() && Date.now() < deadline) { + // 忙等：孙进程若漏成孤儿会一直活着，这里最多等 5s 再断言失败 + execFileSync('sleep', ['0.1']); + } + assert.ok(!alive(), `孙进程 ${gcPid} 仍存活 —— 说明只杀了直接子进程，ocr 派生的进程漏成孤儿`); +}); + +// 整条链的总预算：用完就不再试后面的 provider，宁可本次无评审，也不无限期占着锁。 +test('ocr-review.sh：总预算用尽后不再尝试剩余 provider', skipUnlessBudget, () => { + const sb = mkSandbox(); + // 每一跳都挂到超时；链给 5 条，但预算只够 2 跳左右。 + writeFakeOcr(sb, `#!/usr/bin/env bash\nsleep 300\n`); + fs.writeFileSync( + sb.chain, + ['public|fake|m1', 'public|fake|m2', 'public|fake|m3', 'public|fake|m4', 'public|fake|m5'].join('\n') + '\n' + ); + + const r = runReviewerSync(sb, { OCR_REVIEW_HOP_TIMEOUT: '1', OCR_REVIEW_TOTAL_BUDGET: '3' }); + assert.strictEqual(r.code, 1, '全失败应非零退出'); + // 进度行与「尝试 provider=…」同走 stdout；汇总错误走 stderr。两边合起来看才完整。 + const all = r.stdout + r.stderr; + assert.match(all, /总预算 3s 已用尽，剩余 provider 不再尝试/, '必须明确说明是预算用尽，而非 provider 都坏了'); + assert.match(r.stderr, /3s 预算内没有一跳产出有效评审结果/, '汇总错误里应带上预算，便于区分「链全灭」和「时间不够」'); + // 关键：不能把 5 条全试完（否则预算形同虚设） + const tried = (all.match(/尝试 provider=/g) || []).length; + assert.ok(tried < 5, `预算用尽后不应继续尝试剩余 provider，实际尝试了 ${tried} 跳`); +}); + +// 用量台账：只记「哪个 provider/model、成没成」，供 ocr-pool-status.sh 展示。 +// 绝不写 diff 内容或 key。 +test('ocr-review.sh：成功时把实际使用的 provider/model 记入用量台账', skipUnlessUsageLog, () => { + const sb = mkSandbox(); + const usage = path.join(sb.root, 'usage.log'); + runReviewer(sb, { OCR_REVIEW_USAGE_LOG: usage }); + + const lines = fs.readFileSync(usage, 'utf8').trim().split('\n'); + assert.strictEqual(lines.length, 1, `成功一次只应记一条，实际 ${lines.length} 条`); + const cols = lines[0].split('\t'); + assert.strictEqual(cols.length, 5, '格式应为 时间\\t仓库\\tprovider\\tmodel\\tok'); + assert.strictEqual(cols[2], 'fake'); + assert.strictEqual(cols[3], 'm1'); + assert.strictEqual(cols[4], 'ok'); + // 台账是公共文件，不得泄漏评审内容 + assert.doesNotMatch(lines[0], /示例意见|comments|diff/, '台账只记 provider/model/结果，不得写入评审正文'); +}); + +test('ocr-review.sh：全链失败时也记一条 fail（否则「谁在失败」无从统计）', skipUnlessUsageLog, () => { + const sb = mkSandbox(); + const usage = path.join(sb.root, 'usage.log'); + writeFakeOcr(sb, `#!/usr/bin/env bash +out="" +while [ $# -gt 0 ]; do case "$1" in --output) out="$2"; shift 2;; *) shift;; esac; done +printf '%s\\n' '{"status":"partial","comments":[]}' > "$out" +`); + assert.throws(() => runReviewer(sb, { OCR_REVIEW_USAGE_LOG: usage }), /Command failed/); + const line = fs.readFileSync(usage, 'utf8').trim(); + assert.match(line, /\t-\t-\tfail$/, `全链失败应记 provider=- model=- ok=fail，实际：${line}`); +}); + +// 「链上每个模型都被编码代理占用」不是 provider 故障。record_use 的契约里第三列是 +// ok|skip|fail 三值，这条钉住 skip 真的会被写出来——否则台账把「今天没评审」统计成 +// 「provider 全挂」，用 ocr-pool-status.sh 排查时会一路查错方向。 +test('ocr-review.sh：全被编码代理跳过时记 skip，而不是 fail', skipUnlessUsageLog, () => { + const sb = mkSandbox(); + const usage = path.join(sb.root, 'usage.log'); + // 单跳链 + agent-model 命中该跳的 model → SKIPPED_ALL + fs.writeFileSync(sb.chain, 'public|fake|m1\n'); + fs.writeFileSync(path.join(sb.root, '.opencodereview', 'agent-model'), 'm1\n'); + + assert.throws( + () => runReviewer(sb, { OCR_REVIEW_USAGE_LOG: usage }), + /Command failed/, + '全部被跳过应非零退出（本次没有评审结论）' + ); + const line = fs.readFileSync(usage, 'utf8').trim(); + assert.match(line, /\t-\t-\tskip$/, `被跳过不等于失败，应记 skip，实际：${line}`); +}); + +// 锁回收：cleanup() 的 trap 覆盖了前面那个锁 trap，必须在 cleanup 里再删一次。 +// 否则锁目录永不消失 → 之后每次运行都走「陈旧锁」分支（能接管，但看着像坏了）。 +test('ocr-review.sh：正常运行结束后锁目录必须消失（cleanup 要回收 LOCK_DIR）', skipUnlessLockCleanup, () => { + const sb = mkSandbox(); + const lockDir = path.join(sb.repo, '.git', 'ocr-review.lock'); + // OCR_REVIEW_FORCE='' → 走真实加锁分支（默认的 '1' 会绕过锁，就测不到了） + runReviewer(sb, { OCR_REVIEW_FORCE: '' }); + assert.ok(!fs.existsSync(lockDir), '成功路径结束后锁目录仍存在 —— cleanup 没有回收 LOCK_DIR'); +}); + +test('ocr-review.sh：评审失败退出时同样回收锁目录', skipUnlessLockCleanup, () => { + const sb = mkSandbox(); + const lockDir = path.join(sb.repo, '.git', 'ocr-review.lock'); + writeFakeOcr(sb, `#!/usr/bin/env bash +out="" +while [ $# -gt 0 ]; do case "$1" in --output) out="$2"; shift 2;; *) shift;; esac; done +printf '%s\\n' '{"status":"partial","comments":[]}' > "$out" +`); + assert.throws(() => runReviewer(sb, { OCR_REVIEW_FORCE: '' }), /Command failed/); + assert.ok(!fs.existsSync(lockDir), '失败路径也必须回收锁，否则一次失败会污染后续每次运行'); +}); + +// 静态守卫：把「cleanup 里必须有 LOCK_DIR 回收」钉在实现上，防止后人删掉这一行。 +test('ocr-review.sh：cleanup 必须显式回收 LOCK_DIR（trap 覆盖是这里的历史缺陷）', skipUnlessLockCleanup, () => { + assert.match( + REVIEWER_SRC_TEXT, + /rm -rf \$\{LOCK_DIR:\+"\$LOCK_DIR"\}/, + 'cleanup 的 trap 覆盖了锁 trap，必须在这里再删一次 LOCK_DIR' + ); +}); From b7478386b8eb6219fb89aa10586ef05d2fb0af44 Mon Sep 17 00:00:00 2001 From: Seller-1990 Date: Tue, 6 Oct 2026 16:37:31 +0800 Subject: [PATCH 3/3] =?UTF-8?q?fix(ocr-review):=20FORCE=3D1=20=E5=B9=B6?= =?UTF-8?q?=E8=A1=8C=E8=AF=84=E5=AE=A1=E4=B8=8D=E5=86=8D=E6=8B=86=E6=8E=89?= =?UTF-8?q?=E5=88=AB=E4=BA=BA=E7=9A=84=E9=94=81=EF=BC=9B=E8=A1=A8=E5=A4=B4?= =?UTF-8?q?=E4=B8=8D=E5=86=8D=E6=92=92=E8=B0=8E=E6=9E=9A=E4=B8=BE=E6=A8=A1?= =?UTF-8?q?=E5=9E=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 真实缺陷（本轮自行发现，非 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 行无新增 --- scripts/ocr-review.sh | 30 ++++++++++++++++++------------ test/ocr-review.test.js | 35 ++++++++++++++++++++++++++++++----- 2 files changed, 48 insertions(+), 17 deletions(-) diff --git a/scripts/ocr-review.sh b/scripts/ocr-review.sh index 1cc3f99..3c5b0e6 100755 --- a/scripts/ocr-review.sh +++ b/scripts/ocr-review.sh @@ -7,19 +7,17 @@ # - 也**不定时跑**：只在每个 PR 合并前调用一次。 # # 模型降级链（provider|model，与 ~/.opencodereview/config.json 的 custom_providers 对应）： +# ⚠️ 链的**唯一真源**是 ~/.dsh/templates/ocr-review/chain-public（运行时直接读它）。 +# 本文件里那段 CHAIN 数组只是读不到链文件时的**兜底快照**，别在这里找当前顺序—— +# 它必然滞后（改链不必改脚本，也就没人会记得同步它）。 +# 改链：编辑 chain-public 再跑 bash ~/.dsh/bin/ocr-sync-reviewer.sh 刷快照。 +# 这里不再逐条列举模型名与顺序：链外置的全部意义就是「枚举会过期」，再抄一份 +# 只会制造第二处会撒谎的注释（2026-10-06 实测：注释写着主棒 deepseek，运行时首跳 +# 其实是 chain-public 的 minimax-m3）。各模型的可用性/耗时/成本结论写在链文件里， +# 那才是它们生效的地方。 # 【2026-10-04】隐私铁律暂缓（用户拍板）：私有仓也可用本链（私有副本用 ocr-review.sh.private # 变体，链走 chain-private，同样允许 cloud 条目）。 -# 主 nas-hy4 -> deepseek-v4.1-flash （8787 免费，工具调用已实测） -# 备用① runanytime -> z-ai/glm-5.2 （随时跑路中转，本机中继 127.0.0.1:18789，工具调用已实测） -# 备用② nas-octopus -> glm-5.3-flash （8088 PM-API 免费分组） -# 备用③ nas-hy4 -> hy3 （8787 免费） -# 备用④ nas-hy4 -> hy4-preview-f （8787 免费） -# 备用⑤ x666 -> ministral-14b-latest （云端 0 倍率，工具调用已实测；该站唯一可用，2026-10-04 全站复测） -# 备用⑥ lucky -> stealth/space-bunny-alpha （云端，工具调用已实测） -# 备用⑦ lucky -> step-5-preview （云端，工具调用已实测） -# 备用⑧ daigua -> gpt-6-sol （云端，顾问生产验证过工具调用） -# （lucky-gem/gemini-3.6-flash 已出链：2026-10-04 实测 503 无通道） -# ⚠️ 同站 kimi-k3 不支持工具调用（2026-10-04 三次探针无 tool_calls）→ 不能当 ocr 模型，勿加回。 +# ⚠️ kimi-k3 不支持工具调用（三次探针无 tool_calls）→ 不能当 ocr 模型，勿加回。 # 用法： # ./scripts/ocr-review.sh # 默认基准 origin/dev（不存在则回退 main） # ./scripts/ocr-review.sh v1.8.8 # 对比 tag / 分支 @@ -42,9 +40,14 @@ cd "$REPO_ROOT" || exit 1 # - 持锁进程已死（崩溃/被 kill）→ 视为陈旧锁，接管 LOCK_DIR="$REPO_ROOT/.git/ocr-review.lock" LOCK_INFO="$LOCK_DIR/info" +# 只有**自己拿到锁**才允许在退出时删锁。OCR_REVIEW_FORCE=1 整段跳过、根本不取锁， +# 若无条件删就会把另一个正在跑的评审的锁连根拔掉——而并行评审正是 FORCE 的用途， +# 于是「强制并行」变成「两个进程互相拆锁」。清理由 cleanup() 按这个标记判断。 +LOCK_ACQUIRED="" # OCR_REVIEW_FORCE=1 可绕过（需要并行评审时显式指定） if [ -z "${OCR_REVIEW_FORCE:-}" ]; then if mkdir "$LOCK_DIR" 2>/dev/null; then + LOCK_ACQUIRED=1 printf 'pid=%s\nstarted=%s\n' "$$" "$(date +%s)" > "$LOCK_INFO" trap 'rm -rf "$LOCK_DIR"' EXIT INT TERM else @@ -58,6 +61,7 @@ if [ -z "${OCR_REVIEW_FORCE:-}" ]; then fi # 陈旧锁：持锁进程已不存在，接管 echo "[ocr-review] 发现陈旧锁（pid ${HOLD_PID:-未知} 已退出），接管" + LOCK_ACQUIRED=1 printf 'pid=%s\nstarted=%s\n' "$$" "$(date +%s)" > "$LOCK_INFO" trap 'rm -rf "$LOCK_DIR"' EXIT INT TERM fi @@ -262,7 +266,9 @@ cleanup() { # （dirname 对 "/foo.json" 返回 "/"、对 "foo.json" 返回 "."，**永不为空**，故无需兜底。） fi fi - rm -f "$OUT_JSON" ${ATTEMPT_LOG:+"$ATTEMPT_LOG"}; rm -rf ${LOCK_DIR:+"$LOCK_DIR"} + rm -f "$OUT_JSON" ${ATTEMPT_LOG:+"$ATTEMPT_LOG"} + # 只在**自己持锁**时回收（见 LOCK_ACQUIRED 定义处）：无条件删会把并行评审的锁拆掉。 + [ -n "$LOCK_ACQUIRED" ] && rm -rf "$LOCK_DIR" } trap cleanup EXIT INT TERM diff --git a/test/ocr-review.test.js b/test/ocr-review.test.js index 449651c..9c13e8f 100644 --- a/test/ocr-review.test.js +++ b/test/ocr-review.test.js @@ -43,8 +43,9 @@ const SUPPORTS_DEADLINE = REVIEWER_SRC_TEXT.includes('run_with_deadline'); const SUPPORTS_BUDGET = REVIEWER_SRC_TEXT.includes('OCR_REVIEW_TOTAL_BUDGET'); const SUPPORTS_USAGE_LOG = REVIEWER_SRC_TEXT.includes('OCR_REVIEW_USAGE_LOG'); // 锁清理：cleanup() 的 trap 会覆盖前面那个锁 trap，故必须在 cleanup 里再删一次。 -// 钉住的是「cleanup 里必须出现 rm -rf "$LOCK_DIR"」这一实现事实。 -const SUPPORTS_LOCK_CLEANUP = /rm -rf \$\{LOCK_DIR:\+"\$LOCK_DIR"\}/.test(REVIEWER_SRC_TEXT); +// 但**必须只删自己拿到的锁**：OCR_REVIEW_FORCE=1 整段跳过、根本不取锁，无条件删会把 +// 另一个正在跑的评审的锁连根拔掉——而并行正是 FORCE 的用途。 +const SUPPORTS_LOCK_CLEANUP = /LOCK_ACQUIRED/.test(REVIEWER_SRC_TEXT); const skipUnlessChainFile = SUPPORTS_CHAIN_FILE ? {} @@ -623,11 +624,35 @@ printf '%s\\n' '{"status":"partial","comments":[]}' > "$out" assert.ok(!fs.existsSync(lockDir), '失败路径也必须回收锁，否则一次失败会污染后续每次运行'); }); -// 静态守卫：把「cleanup 里必须有 LOCK_DIR 回收」钉在实现上，防止后人删掉这一行。 -test('ocr-review.sh：cleanup 必须显式回收 LOCK_DIR（trap 覆盖是这里的历史缺陷）', skipUnlessLockCleanup, () => { +// 并行评审的核心用例：OCR_REVIEW_FORCE=1 表示「我知道有锁，我偏要并行跑」， +// 它整段跳过取锁。此时若 cleanup 仍无条件 rm -rf "$LOCK_DIR"，就会把**另一个 +// 正在跑的评审**的锁连根拔掉——FORCE 的用途恰恰是并行，于是变成互相拆锁， +// 两个进程都以为自己独占。这条先证伪再锁定：pre-fix 脚本跑完锁会消失。 +test('ocr-review.sh：FORCE=1 并行运行时不得删掉别人的锁', skipUnlessLockCleanup, () => { + const sb = mkSandbox(); + const lockDir = path.join(sb.repo, '.git', 'ocr-review.lock'); + // 模拟「另一个评审正持锁」：目录存在 + info 里写一个活着的 pid（用本进程， + // 保证 kill -0 为真，走不到「陈旧锁接管」分支——否则就测成接管而非并行了）。 + fs.mkdirSync(lockDir, { recursive: true }); + fs.writeFileSync(path.join(lockDir, 'info'), `pid=${process.pid}\nstarted=0\n`); + + runReviewer(sb, { OCR_REVIEW_FORCE: '1' }); + assert.ok( + fs.existsSync(lockDir), + 'FORCE=1 没有取锁，退出时却把锁删了 —— 并行评审会互相拆锁' + ); +}); +// 同时钉住**有条件的**回收——无条件 rm -rf 会让 FORCE=1 的并行评审互相拆锁。 +test('ocr-review.sh：cleanup 回收 LOCK_DIR 必须限定在自己持锁时', skipUnlessLockCleanup, () => { assert.match( + REVIEWER_SRC_TEXT, + /\[ -n "\$LOCK_ACQUIRED" \] && rm -rf "\$LOCK_DIR"/, + 'cleanup 的 trap 覆盖了锁 trap，必须在这里回收 LOCK_DIR；且必须限定 LOCK_ACQUIRED，否则拆掉并行评审的锁' + ); + // 反面：不得再出现无条件的 rm -rf "$LOCK_DIR"（FORCE=1 路径从未取锁） + assert.doesNotMatch( REVIEWER_SRC_TEXT, /rm -rf \$\{LOCK_DIR:\+"\$LOCK_DIR"\}/, - 'cleanup 的 trap 覆盖了锁 trap，必须在这里再删一次 LOCK_DIR' + 'FORCE=1 不取锁却删锁 = 拆掉并发评审的锁，这个写法已废弃' ); });