Files
Agent-System-Dev/.task/reviews/2026-10-20-svc-pipefail-independent-review.md
T

13 KiB
Raw Blame History

独立代码审查:fix/svc-pipefail-early-exit-pipes(svc 管道早退读取器治理)

  • 审查对象:7e19cf3e182b566bdc48a7e690e44ff11b4ab33c..f87d63ffd8ece163bd7054c80cb51ccd6159f994
  • 涉及文件:scripts/svc.sh、scripts/task.sh、scripts/e2e.sh、scripts/lint-boundaries.sh、docker/verify-env.sh、tests/load/collaboration/run_pgbench.sh、新增 tests/test_shell_pipe_hazard.py(另含任务卡 .task/fix-svc-pipefail-early-exit-pipes.md,属 .task/** 豁免范围)
  • 审查者身份:全新会话独立审查,未参与实现;本会话仅写入本报告,其余只读
  • 参照先例:Dev PR #657(5562015,scripts/task.sh 回收路径 git ls-files --stage | grep -q '^160000 ' → awk 全量消费,commit 8557ee9/daf0288..8557ee9)

结论:PASS(无阻塞项;4 项非阻塞建议)

判定标准逐项核对:无语义回归、无判定翻转、守护测试非形同虚设(真实可失败、对已修复形态全部拦截)、无二进制误判。


一、逐处语义等价核对(重点 1)

六个脚本均声明 set -euo pipefail(svc.sh:134、task.sh:4、e2e.sh:24、lint-boundaries.sh:4、verify-env.sh:2、run_pgbench.sh:10),隐患前提成立。共 34 处变更,按变换形态归类核对:

A. 变量 → here-string(grep … <<<"$var",共 15 处)

sites:verify-env.sh:34、48;lint-boundaries.sh:76;task.sh:23、26、29、214、219、315、316、1070;svc.sh:1067、1703、3577、4923、4955、5040、5100。

原形均为 echo/printf "$var" | grep … 或 printf '%s' "$var" | head -c N。实测(/tmp/pipefail-review):here-string 会在串尾补 \n,echo/printf '%s\n' 同样补 \n;printf '%s' 不补但 grep 按行匹配对尾随换行不敏感,且 task.sh:1070 经命令替换取值时尾随 \n 被剥除——实测 $(head -c 120 <<<"$short") 与 $(printf '%s' "$short" | head -c 120) 输出逐字节一致(含 <120 字节与 >120 字节两例)。命中/未命中/多行/退出码全部一致。此处原"生产者"是 shell 内建 echo/printf,不存在可丢失的生产者退出码。等价,成立。

B. 文件输入 sed:sed -n 's/^K=//p' f | head -1 → sed -n '/^K=/{s///p;q;}'(共 17 处)

sites:svc.sh:900、906、1067、2432、3320、5449、5506-5509、5676、5939、5977、6086;task.sh:643、644、647、648、649;run_pgbench.sh:186。

实测(GNU sed 4.9,/tmp/pipefail-review/kv.txt 多值键/无键/空值用例):多行匹配取首行、未命中输出空、k=(空值)输出空——新旧输出一致;s/// 空正则复用地址正则(POSIX 语义),替换结果与原 s/^K=//p 一致。退出码:未命中旧管道(pipefail)=sed 的 0=新式 0;文件不可读旧=2(pipefail 传播 sed)=新式 2。sed q 只在匹配后退出,输入是文件而非管道,无 SIGPIPE 面。等价,成立。

C. 全量消费替代早退:| head → | sed -n 1p / sed -n '1,12p'(3 处)

  • verify-env.sh:41:find … | sed -n 1p——sed 无 q 会读尽输入,输出首行,实测与 head -n 1 输出一致(含空输入)。
  • run_pgbench.sh:180:ps … | sed -n '1,12p'——同上,实测输出一致。

D. 进程替换 < <(producer)(6 处)

svc.sh:1476(find -print -quit)、3943(strings)、4415(pg_psql)、4949(serve -h)、5087(pg_run)、5156/5169(head -c 15)。退出码语义变化见第二节。

E. e2e.sh:211:grep -o '装载(.*)' f | head -1 || true → grep -m1 -o … || true

实测:.* 贪婪使每行至多产生一个匹配,故 head -1(第一个匹配)≡ -m1 -o(首个匹配行的全部匹配)——单行/单匹配/整行贪婪匹配三例输出一致。文件输入无管道 SIGPIPE;|| true 为原有写法(旧式同样以 || true 收尾),非新增掩盖。等价,成立。


二、退出码语义的真实变化逐处判定(重点 2)

进程替换后生产者退出码不再参与判定。前提复现:bash -c 'set -euo pipefail; seq 1 200000 | grep -q 1' 实测判为 MISS、裸管道状态 141(隐患为真)。「生产者失败但输出恰好命中」实测确实由旧=失败翻转为新=成功(f(){ echo X; return 3; }:旧管道判 FAIL(3),新 procsub 判 HIT)。逐处判定:

site 原行为 新行为 判定
svc.sh:4949 serve -h 2>&1 | grep -Eq config -h 以非零退出(真实 CLI 常见)时 pipefail 使判定翻为「不识别」→ 假警报 仅看 help 文本是否含 --config(实测 2×2 矩阵:exit0+命中=静默、exit0+未命中=告警、exit2+命中=旧假警报/新正确静默、exit2+未命中=告警) 修正。判定目标本就是「help 文本内容」,忽略 -h 退出码语义正确;未发现有害翻转场景
svc.sh:3943 strings | grep -q AGENT_SYSTEM_DSN 大二进制(串靠前、残余 >64KiB)时 strings 吃 SIGPIPE→141→&& return 0 失败→假「不支持」(实测复现:合成用例旧判 NOT supported) 命中即真支持;strings 半途 I/O 失败且部分输出恰含该串才会假「支持」——部分输出含串即证明产物确实内嵌该串 修正(PR #657 同类缺陷的真实翻版)
svc.sh:4415/5087 pg_psql/pg_run "SELECT 1 …" tA | grep -q 1 psql 连接失败(空输出)→ 判「库不存在」→ 转 CREATE → 二次失败报错(误导);「失败但 stdout 恰有 1」对单值 SELECT 不成立 失败(空输出)同样判「不存在」转 CREATE,下游可见失败,与旧一致 中性偏修正;无真实「失败但命中」场景
svc.sh:1476 find … -print -quit | grep -q . 单行输出,无缓冲面;目录不存在时新旧均判空(实测均 status 1) 同 中性(防御性)
svc.sh:5156/5169 head -c 15 | grep -qa head 只写 15 字节(<64KiB),无 SIGPIPE 面;文件可读性由前置 [ -f ] 保证 同 中性
verify-env.sh:41、run_pgbench.sh:180 find/ps 输出 >64KiB(大型 playwright 安装、>~700 进程)或 ps 偶发 exit 1 时,旧式在 set -euo pipefail 下直接中止(141 传播) sed 读尽输入,状态恒 0,失败不再中止诊断/汇总块 修正;ps 失败被掩盖属诊断块可接受(已在旧代码 flaky 风险与本次修复间取舍,记录在案)
其余 here-string/sed 文件输入各处 — 生产者为内建或 sed 自身,无生产者退出码可丢失 等价

结论:所有真实变化方向均为「修正旧判定翻转」或「中性等价」,未发现「生产者失败但输出命中被新写法误判成功且旧写法正确拒绝」的成立场景。

三、二进制与边界(重点 3)

svc.sh:5156/5169 grep -qa '^SQLite format 3$' < <(head -c 15 …):

  • 进程替换是 fd 直通,不经过 bash 变量,无 NUL 截断面;-qa 保留,按字节读取(实测 head -c 15 real.db | od -c = S Q L i t e f o r m a t 3,15 字节,NUL 是第 16 字节本就不在窗口内)。
  • 无尾随换行的"末行"命中语义新旧一致:实测真 SQLite 头两式均检出、非 SQLite 文件两式均不检出。
  • head -c 15 产出 ≤15 字节,新旧均无 SIGPIPE 面。无回归。

四、守护测试质量(重点 4)

tests/test_shell_pipe_hazard.py(311 行):词法扫描器 + 4 个自测。

  • 真实可失败:PYTHONDONTWRITEBYTECODE=1 python3 -m unittest tests.test_shell_pipe_hazard -v → 4 tests OK;检测器对合成隐患 seq 1 200000 | grep -q 1、if ! … | grep -q、sed|head 命令替换、-m1、续行管道、sed q、grep -qiE || exit 七种形态全部命中(test_detector_flags_* 断言存在,检测器失效即测试红)。
  • 注释/heredoc/case/引号覆盖:实测探针 14 项全过——注释行、heredoc 正文、case 模式 |、双引号内管道文本均不误报;<<<、< <()、sed -n 1p 安全形态不误报;grep -q/-m、head、sed q 尾级全部命中。
  • 已知盲区(实测确认,当前树未触发恶性形态):
    1. 只查「管道末级」:中段早退读取器不查——svc.sh:3804/5482 grep -oE '"dsn"…' | head -1 | sed -E …(两处)原样存留且守护通过;
    2. 不建模 awk/read:svc.sh:1058 ss -ltnp | awk '{print; exit}' 为尾级早退读取器,未命中;
    3. 不识别前缀化 grep:producer | LC_ALL=C grep -q x、command grep -q 可绕过(当前六脚本无此写法,实测确认)。
  • 311 行是否失衡:判定为可接受、不失衡。svc.sh 含 heredoc、case 模式、嵌套 $()、续行管道与多种引用,逐行正则式简化实现必然双向误报/漏报(其漏报恰是本缺陷类);扫描器自带合成自测可失败,复杂度换来的是零误报地通过当前树。失衡风险的代价是上述盲区,见非阻塞建议。

五、范围与遗漏(重点 5)

  • 变更范围:git diff --name-only 7e19cf3..f87d63f 恰为 7 个目标文件 + 任务卡(.task/** 豁免),无越界改动。
  • 残留早退读取器(runtime/ 已确认为 ignored 陈旧克隆:git check-ignore -v runtime/ 命中 .gitignore:9,且其下 0 个 tracked 文件,未据此判定):
    • 受管脚本内(先于本分支存在,git show 7e19cf3 证实):svc.sh:3804、5482(中段 head -1)、svc.sh:1058(尾级 awk … exit)。现实触发需 >64KiB 的 dsn 匹配行/海量监听套接字,且失败形态是 $(…) 赋值非零→set -e 中止(响亮失败),非静默翻转——不阻塞。
    • 受管清单外(本单元范围外,仅记录):e2e-duties.sh:87(ps -p | grep -Fq,单行输出,bounded)、:128(curl | head -c 160);tests/integration/rdm-increment/acceptance_scenarios.sh(set -u 无 pipefail,echo 内建生产者);tests/ops/lib.sh:82、146(无 pipefail)。
  • 掩盖写法:六脚本无 set +o pipefail;本分支新增的 || true 仅 e2e.sh:211 一处且为原有收尾写法的保留,非掩盖新增。PR #657 式「全量消费」替代(awk)未用于本批,改用 here-string/procsub/sed-q,方向一致且更彻底。

非阻塞建议(不阻塞合入)

  1. 守护测试盲区:把检测范围扩到中段读取器(| head、| sed q 出现在任意非首段即报),并建模 awk '…exit'、| read、前缀化 grep(LC_ALL=C grep);或至少在 docstring 声明「仅末级」契约的同时为 svc.sh:3804/5482/1058 立即整改。当前两处 dsn 残留建议改 sed -n '/"dsn"/{s//…/p;q;}' 式文件直读。
  2. SCRIPTS 元组硬编码:新增受管脚本不会被自动纳入扫描,建议 glob(如 scripts/*.sh + 白名单豁免)。
  3. 守护测试纯词法:可加一条行为级自检(bash -c 'set -o pipefail; seq 1 200000 | grep -q 1; test $? -eq 141')把 141 前提钉进测试,防 bash 语义漂移。
  4. run_pgbench/verify-env 的 sed 替代掩盖了 ps/find 的真实失败状态:诊断块可接受,但若未来该块内命令失败需要可见,应显式记录状态。

已运行的验证命令与结果

命令(只读) 结果
git log --oneline 7e19cf3..f87d63f / git diff --stat 同范围 2 commits;8 文件(7 目标 + 任务卡),+448/−47
git status --porcelain(开工时) 干净;HEAD=f87d63ff(detached)
六脚本 set - 声明检查 全部 set -euo pipefail(svc.sh:134 等)
/tmp/pipefail-review 语义实验(sed 新旧式多值/未命中/空值/不可读;here-string vs printf 含 head -c 120;SIGPIPE 复现 141;procsub 退出码翻转矩阵;serve -h 2×2 矩阵;grep -m1 -o vs head -1 贪婪匹配;SQLite 头 15 字节两式对照;find -quit 有/无目录) 全部如上文引用,行为与判定一致
PYTHONDONTWRITEBYTECODE=1 python3 -m unittest tests.test_shell_pipe_hazard -v 4 tests OK(0.083s)
PYTHONDONTWRITEBYTECODE=1 python3 -m unittest tests.test_svc tests.test_svc_backup 34 tests OK(34.3s)
检测器 14 项探针(含中段 head、awk exit、env 前缀 grep、read、注释/heredoc/case/引号) 14/14 符合设计;盲区 3 类确认(见第四节)
22 个 tracked *.sh 独立残留扫描(grep 形态匹配) 残留清单见第五节
bash -n 六脚本 全部 OK
git check-ignore -v runtime/;git ls-files runtime/ | wc -l ignored(.gitignore:9);0 个 tracked 文件

未验证项(如实标注)

  • 仅在 GNU sed 4.9 / 当前 bash 验证;macOS BSD sed 的 s/// 复用地址正则与 {…q;} 行为未验证。
  • strings 大二进制路径为合成复现,未在真实制品上运行(NOT_RUN)。
  • 完整测试套件与 CI 未运行(本单元只读约束内的过滤子集已全绿)。
  • e2e-duties.sh、acceptance_scenarios.sh、tests/ops/lib.sh 的残留未整改(范围外,仅登记)。