agent-exec transport ef1c90b2b8b15ee0cd97fe1c244db151/eef52bc713d1
This commit is contained in:
agent-exec-transport
@@ -0,0 +1,97 @@
|
||||
# 独立代码审查：task.sh finish 的 gitlink 检测 pipefail 修复
|
||||
|
||||
- unit_id：`tasksh-force-independent-review`
|
||||
- 审查对象：`8a8f9f57a58b4ec8701b576a98c5f0c0c5be5cd5..d755fa0f6d112b6cc488b60d937d8419696e1edb`（范围内唯一提交 d755fa0，仅 `scripts/task.sh` +3/-1 与 `tests/test_task.py` +20）
|
||||
- 审查者身份：全新独立会话，未参与实现；除本报告文件外未修改任何文件，未执行 git add/commit/push。
|
||||
- 契约依据：policy/governance.md §回收（L141-143「Git 对含子模块的工作区需要显式删除选项时，只有全部现场检查通过才允许该路径；它不豁免任何脏状态……普通删除失败不转入强删兜底」、L145「远端分支删除与本地回收分别报告」）；policy/delivery.md §清理与恢复（L449-451）。
|
||||
|
||||
## 结论：PASS
|
||||
|
||||
无阻塞项。修复正确、意图保持完整、回归测试真实复现生产故障且在未修复脚本上失败、范围未越界。非阻塞建议见文末。
|
||||
|
||||
---
|
||||
|
||||
## 重点 1：修复正确性 —— 新检测式
|
||||
|
||||
变更（scripts/task.sh:1133-1136）：
|
||||
|
||||
```bash
|
||||
# Read the whole listing: an early-exit reader SIGPIPEs git on large indexes and pipefail drops --force.
|
||||
if git ls-files --stage | awk '$1 == "160000" { found = 1 } END { exit !found }'; then
|
||||
removal+=(--force)
|
||||
fi
|
||||
```
|
||||
|
||||
依据与结论：
|
||||
|
||||
1. **读取全部输出、与索引大小无关**：awk 主规则只赋值不 exit，天然消费到 EOF。实测本工作区（822 行索引）`git ls-files --stage | awk '{count++} END {print count}'` 输出 822，与 `wc -l` 一致。
|
||||
2. **仅索引含 mode 160000 时为真**：`ls-files --stage` 每行格式 `<mode> <oid> <stage>\t<path>`，`$1` 恒为 mode（路径在 TAB 之后，含空格/引号路径不影响 `$1`；git 对含换行等控制字符的路径按 core.quotePath 默认 C-quoted 输出，不产生裸换行行）。`160000` 只有 gitlink；普通文件 100644/100755、符号链接 120000 均不匹配。冲突 stage 1/2/3 的 gitlink 行 `$1` 同为 160000，与旧 `grep '^160000 '` 语义一致。
|
||||
3. **真值等价且消除 SIGPIPE**：`END { exit !found }` 在 found=1 时 exit 0、否则 exit 1，与 `grep -q` 的 0/1 真值一致；差异仅在于不再早退。
|
||||
4. **两种仓库实测**（临时目录 /tmp/review-gitlink-GNWq，`git init` + 3000 个文件 ≈178KB listing）：
|
||||
- 无 gitlink：新检测式 exit=1（不加 `--force`，正确）。
|
||||
- 有 gitlink 且排在前、其后 178KB 输出：新检测式 exit=0；同一索引上旧式 `git ls-files --stage | grep -q '^160000 '` 在 `set -o pipefail` 下 pipeline exit=**141**（git 吃 SIGPIPE）——即生产故障机理的直接复现，证明修的是真问题。
|
||||
5. **可移植性**：`$1 == "160000"`、变量赋值、`END { exit expr }` 均为 POSIX awk；gawk/mawk/busybox awk/BSD awk 通吃。bash -n 通过。shellcheck 未安装，未运行（如实标注 NOT_RUN）。
|
||||
|
||||
## 重点 2：意图保持 —— 门禁未被削弱
|
||||
|
||||
逐条对照 d755fa0 的 cmd_finish（scripts/task.sh:1092-1166），diff 仅替换 L1135 一行检测式与新增 L1134 注释，其余全部未动：
|
||||
|
||||
- 写者确认：L1095 `TASK_WRITERS_STOPPED=1` 原样。
|
||||
- 分支类型门禁：L1096-1097（symbolic-ref + feature/fix/hotfix/release 白名单）原样。
|
||||
- 合入祖先校验：L1116 `merge-base --is-ancestor`（含 immediate 父任务解析 L1110-1114）原样。
|
||||
- 远端 lease：L1117 `ls-remote` → L1119 fetch → L1120 rev-parse FETCH_HEAD → L1122 `--is-ancestor`，原样。
|
||||
- fetch 后二次 `check_worktree finish`（registered-root/owner/锁/递归脏检查）：L1132 原样；`--force` 仍只在全部现场检查通过后的这一个分支点追加（governance §回收「只有全部现场检查通过才允许该路径」得到保持）。
|
||||
- Git 拒绝时不得 rm 兜底或追加 force 等级：L1138-1139 `die "…no rm fallback or additional force level is permitted."` 原样。
|
||||
- 分支与远端分别报告：L1152-1164 的 `[recycled]/[partial]` 四分支与 `--force-with-lease` 删除租约（L1156）原样。
|
||||
- **无误判 force**：无 gitlink 仓库实测检测式为假（重点 1.4），不会给普通仓库追加 `--force`；有 gitlink 时追加行为与修复前语义一致（只是不再被 SIGPIPE 误翻为假）。
|
||||
|
||||
## 重点 3：回归测试有效性（tests/test_task.py:496-515）
|
||||
|
||||
1. **复现生产故障机理**：夹具含 gitlink（`component`）+ 其后 2000 个 `zz-bulk/entry-*.60字符` 文件（每行 listing ≈111B，共 ≈217KB）。gitlink 行在 `zz-bulk` 之前由 'c'(0x63) < 'z'(0x7a) 的字节序决定——测试不是靠假设，而是用 `split("\tcomponent\n")` + `before_gitlink.rsplit("\n",1)[-1].startswith("160000 ")`（L510）显式断言了 gitlink 行位置，排序假设被破坏时测试会响亮失败而非静默失效。
|
||||
2. **64KiB 断言稳健**：L512-513 断言全 listing >64KiB、gitlink 之后 >2×64KiB。>2× 的余量覆盖默认 64KiB 管道缓冲 + grep 单次读缓冲的提前消费，在 Linux 默认管道容量（64KiB）下判定确定性充分。残余风险：若某环境的管道容量或读者缓冲合计超过 ~217KB（非默认配置），旧脚本可能不再触发 SIGPIPE——见非阻塞建议 N1。
|
||||
3. **修复前必失败（已实测）**：在 /tmp 一次性 clone（不触碰本工作区）中，将 scripts/task.sh 还原为 `git show 8a8f9f5:scripts/task.sh` 后运行该用例：**FAIL**，失败点即生产故障原文——`fatal: working trees containing submodules cannot be moved or removed` → `ERROR: Git refused worktree removal; … no rm fallback …`，exit 1≠0。同一 18 个 `-k finish` 用例在旧脚本下恰好只有这 1 个失败，证明该用例是唯一判别用例、修复未改变其它 finish 行为（旧的 `test_finish_recycles_clean_submodule_without_shared_config_changes` 因小索引不触发 SIGPIPE，在新旧脚本下均通过——这正是该缺陷此前潜伏的原因）。
|
||||
4. **修复后通过（已实测）**：本工作区运行该用例 `ok`（0.512s）；`-k finish` 全部 18 用例 `OK`（10.108s）。
|
||||
5. **无偶发/网络依赖**：夹具远端是本地 bare 仓（push 为本地文件传输），triage 桩不参与 finish 路径；同一用例在两个环境共运行 4 次全部稳定。
|
||||
|
||||
## 重点 4：范围与规范
|
||||
|
||||
- `git diff --stat` 范围内仅 `scripts/task.sh`（+3/-1）与 `tests/test_task.py`（+20），无其它文件，无隐性扩围。
|
||||
- 风格一致性：仓库已有同类先例——task.sh:113-114（`card_merged_into`「以完整消费输出取首行，避免 grep -q 早退令 git log 吃 SIGPIPE（141 under pipefail）」）。本次修复（全文消费 + 英文注释说明 SIGPIPE 机理）与该先例同构，注释语言与 cmd_finish 周边英文注释一致；测试注释与相邻 finish 用例一致。
|
||||
- `set -euo pipefail`（task.sh:4）是缺陷前提，本次未改动全局 set，改动局部于单行检测式，最小充分。
|
||||
|
||||
## 重点 5：残余风险（非阻塞，不在本任务修复）
|
||||
|
||||
`grep -rnE "\| *grep -q" scripts/` 排除内建生产者（printf/echo）后，`set -euo pipefail` 的 scripts/svc.sh 中存在同类「外部生产者 + 早退读者」管道：
|
||||
|
||||
- **svc.sh:3943** `strings "$1" 2>/dev/null | grep -q 'AGENT_SYSTEM_DSN' && return 0`：大二进制上 strings 输出可超 64KiB，匹配靠前时 strings 吃 SIGPIPE →141 → `&&` 不成立 → 携带 DSN 的二进制被误判为无 DSN。与本次修复的缺陷同类，最值得后续独立任务处理。
|
||||
- **svc.sh:1476** `find "$backup" … -print -quit | grep -q .`：双侧早退存在理论竞态，但单行输出远小于管道缓冲，实际风险低。
|
||||
- svc.sh:4415/5087（psql `-tA` 单行输出）、5156/5169（`head -c 15` 有界）实际安全。
|
||||
|
||||
以上按单元要求仅记录为非阻塞建议，本任务不修。
|
||||
|
||||
## 已运行的验证命令与结果
|
||||
|
||||
| # | 命令 | 结果 |
|
||||
|---|---|---|
|
||||
| V1 | `git diff --stat 8a8f9f5..d755fa0` | 仅 scripts/task.sh +3/-1、tests/test_task.py +20 |
|
||||
| V2 | `git show 8a8f9f5:scripts/task.sh`（对照 L1134 旧式 grep -q） | 旧检测式确认为 `git ls-files --stage \| grep -q '^160000 '` |
|
||||
| V3 | `bash -n scripts/task.sh` | 通过 |
|
||||
| V4 | `which shellcheck` | 未安装 → shellcheck NOT_RUN |
|
||||
| V5 | 临时仓（/tmp/review-gitlink-GNWq）：无 gitlink 跑新检测式 | exit=1（不加 force） |
|
||||
| V6 | 同仓加入 gitlink（排前 + 178KB 后续输出）跑新检测式 | exit=0 |
|
||||
| V7 | 同仓 `bash -c 'set -o pipefail; git ls-files --stage \| grep -q "^160000 "'` | exit=141（SIGPIPE，生产机理复现） |
|
||||
| V8 | 同上换新检测式并读 PIPESTATUS | pipeline exit=0、git exit=0（全量消费） |
|
||||
| V9 | 本工作区 `git ls-files --stage \| awk`（工作区含 gitlink） | exit=0；awk 行数 822 == wc -l 822 |
|
||||
| V10 | `PYTHONDONTWRITEBYTECODE=1 python3 -m unittest tests.test_task.TaskLifecycleTests.test_finish_recycles_submodule_when_index_listing_exceeds_pipe_buffer -v`（工作区=修复后） | ok，Ran 1 test in 0.512s |
|
||||
| V11 | `… -k finish -v`（工作区） | Ran 18 tests, OK (10.108s) |
|
||||
| V12 | /tmp 一次性 clone + `git show 8a8f9f5:scripts/task.sh > scripts/task.sh` 后运行新用例 | FAIL：`fatal: working trees containing submodules cannot be moved or removed` → exit 1≠0（生产故障原文） |
|
||||
| V13 | 同 clone `… -k finish` | Ran 18 tests, FAILED (failures=1)，唯一失败即新用例 |
|
||||
| V14 | `git status --short`（收尾复核） | 空（工作区除本报告外无修改） |
|
||||
|
||||
未验证事项：shellcheck 静态分析未运行（工具未安装）；>2×64KiB 管道余量仅覆盖 Linux 默认管道容量（V10/V12 在本机 Linux 通过/判别成立），非默认管道容量的 CI 环境未实测（见 N1）。
|
||||
|
||||
## 非阻塞建议
|
||||
|
||||
- N1（测试微硬化，可选）：`assertGreater(len(after_gitlink.encode()), 2 * 64 * 1024)` 可再留一档余量或对夹具条目数写明容量假设；当前 2× 在 Linux 默认 64KiB 管道下已充分（V10/V12 实证），不改不阻塞。
|
||||
- N2（后续独立任务）：修复 svc.sh:3943 `strings | grep -q` 的同类 pipefail 早退隐患（全量消费或去掉早退读者）。
|
||||
- N3（后续独立任务）：评估 svc.sh:1476 `find -print -quit | grep -q` 的双侧早退竞态（当前实际风险低）。
|
||||
Reference in New Issue
Block a user