diff --git a/.task/reviews/2026-10-20-svc-pipefail-independent-review.md b/.task/reviews/2026-10-20-svc-pipefail-independent-review.md new file mode 100644 index 00000000..c06e7616 --- /dev/null +++ b/.task/reviews/2026-10-20-svc-pipefail-independent-review.md @@ -0,0 +1,116 @@ +# 独立代码审查: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 的残留未整改(范围外,仅登记)。