diff --git a/.task/reviews/2026-10-20-svc-flake-review.md b/.task/reviews/2026-10-20-svc-flake-review.md new file mode 100644 index 00000000..b9c039c3 --- /dev/null +++ b/.task/reviews/2026-10-20-svc-flake-review.md @@ -0,0 +1,73 @@ +# 独立代码审查：fix/svc-static-deploy-test-port-flake（static 首启用例硬编码端口致偶发失败） + +- 审查对象：packet 指定为 `5c3a1d4eb8f3a5c0c0c5be5cd5`（implementation 的 transport commit `d0b217d3a486d32db91d69539c39d07ed9dbdae0`）相对同步后 base `2fd11effd9ccb0ae35ae5a47caccefccbbb05a8e` 的 diff。**对象勘误（事实陈述，见 N1）**：`5c3a1d4e…` 在本工作区不存在（`git cat-file -t` → fatal）；transport commit `d0b217d3` 的父提交是任务卡提交 `409f4749`，其分支自 R0=`a9566a68`（01:58 的 main）切出，而 `2fd11ef` 是 09:30 的 main——两者互不为祖先（`git merge-base --is-ancestor` 双向否定，merge-base=`a9566a68`）。因此 `git diff 2fd11ef d0b217d3` 含约 2400 行与本单元无关的主线演进内容（tests/test_shell_pipe_hazard.py 重写、e2e-duties.sh、hooks/pre-commit、svc.sh:4360、modules/backend gitlink、删除一份 spec 文档）；**本单元自身 diff（`git diff a9566a68 d0b217d3` = `git diff 409f4749 d0b217d3`）只有**：`.task/fix-svc-static-deploy-test-port-flake.md`（host 豁免路径）+ `tests/test_svc.py` 单 hunk（3 增 1 删，第 989 行 `port = 18951` → `with socket.socket() as reservation: bind(("127.0.0.1", 0)); port = reservation.getsockname()[1]`）。产品侧 `scripts/svc.sh` 本单元零改动（d0b217d3 与当前 HEAD 2841cb4 在 svc.sh 的 1 行差异是主线管道治理提交所致，位于 `db_create` 的 postgres 路径，本用例不执行）。 +- 审查者身份：全新会话独立审查，未参与实现；本会话仅写入本报告，其余只读。 +- 被测代码：当前 HEAD `2841cb4` 下 `tests/test_svc.py` 与 `d0b217d3` 版本逐字节一致（`git diff d0b217d3 2841cb4 -- tests/test_svc.py` 为空），故只读测试在被测 HEAD 上运行即覆盖 transport commit 的测试代码。 + +## 结论：PASS（无阻塞项；4 项非阻塞建议） + +判定标准逐项核对：用例语义未被削弱（9 条断言逐条与 base 相同，仅端口来源一行变化）；TOCTOU 未引入可静默通过的偶发失败（失效模式是 `check_port_free` 报错退出 → `self.svc(check=True)` 断言带全量 stdout 响亮失败）；受保护内容（`scripts/svc.sh`、`tests/test_svc.py:753-754` register_pg 夹具、测试类结构）未被改；补丁未外溢到其他用例或脚本（单文件单 hunk，实测 `git diff --stat a9566a68..d0b217d3` 仅 2 文件且其一为 `.task/**`）。 + +--- + +## 重点 1：端口分配语义与 TOCTOU 窗口 + +**写法**：`socket.socket()`（默认 AF_INET/SOCK_STREAM）→ `bind(("127.0.0.1", 0))` 由内核在临时口范围内分配 → `getsockname()[1]` 取实际端口 → `with` 块退出即 close。与本仓 5 处既有夹具逐字节同构：`tests/test_svc.py:366-368`（同类同类）、`tests/test_svc_recover.py:187-190`、`tests/test_task_pr_check.py:100-102`、`tests/test_task_triage.py:230-232`、`tests/test_svc_realcombo.py:128-130`（另 `tests/test_ci_wait.py:76`）。属既有惯例，非新引入模式。 + +**TOCTOU 窗口评估**（分析 + 局部实测）：reservation close 到 `svc.sh` 内 `python3 -m http.server` 真正 bind 之间存在窗口，内核不为已关闭的监听套接字保留端口。实践中风险可忽略： +- 本机临时口范围 `32768-60999`（`/proc/sys/net/ipv4/ip_local_port_range` 实测），与套件所有固定端口（188xx/189xx/947x）**不相交**，动态端口不可能撞上兄弟用例；碰撞只可能来自无关进程在同一瞬间抽中同一临时口。 +- 「服务主动保留端口」环境（`ip_local_reserved_ports`）：内核 `bind(0)` 分配本就跳过保留段，分配步不会拿到保留口，不构成偶发源【分析推断，未单独实测】。 +- 「worker 池已尽/临时口耗尽」：`bind(0)` 自身抛错 → 用例报错，属环境问题的响亮失败，不会假绿【代码路径推断，未单独复现】。 +- **决定性实测（run 2）**：外部监听器占住旧硬编码口 18951 期间，目标用例 PASS（0.518s）——修复前该场景确定性失败（任务卡 host 复现 `AssertionError: 1 != 0`）。 + +**外部进程（如 Node）占口后的可观察性**：若抽中的端口在 `check_port_free`（scripts/svc.sh:1575）时已被占，static 部署报「端口被占用（非本服务）：…」并退出非零 → `self.svc(check=True)` 的 `assertEqual(returncode, 0, stdout)` 失败并打印全量 stdout，占用者 pid 可查（`port_owner`）。比修复前更可归因：从「神秘硬编码口撞车」变为「带占用者信息的明确冲突」。 + +## 重点 2：用例失败可观察性是否保留 + +对比 base（a9566a68）与 d0b217d3 的用例方法体：`self.assert*` 计数 9=9，逐条未删未弱化（assertIn「已启动」、assertNotIn「进程即退」、assertEqual kind=static、assertIsNotNone pidcheck、assertIn `--directory`、assertIn resolve 后 htdocs、assertTrue pid_alive、assertEqual 200、assertIn static-ok）。仍然可见的失败类：(a) 端口冲突 → 部署退出非零 + stdout 全文；(b) pidcheck 登记回归 → assertIsNotNone/assertIn；(c) 首启探活误判 → assertNotIn「进程即退」；(d) http.server 未起/内容不符 → 部署失败或 urlopen 断言失败。唯一语义变化是「端口被他者占用时不再假失败」——这正是修复目标，不是可观察性损失。 + +## 重点 3：不改 register_pg（:753-754）是否合理 + +**合理，且为契约要求**。实测 `register_pg`（tests/test_svc.py:231-246）仅把 kind/env/addr/tags/source/version/user[/password]/dbname/authmode/pgdata 各行 `write_text` 进 `runtime//svc.env`，从不创建或绑定任何 socket；`:753-754` 的 `18951` 只是夹具元数据，被 stub 以 DSN 字符串消费。它不可能引发端口占用型偶发失败，不改正确。修复后全仓跟踪文件中 `18951` 仅剩这两行（`git grep` 实测）。 + +## 重点 4：其他用例是否仍硬编码 18951 / 是否需同改 + +`18951` 已无其他出现点。其他用例确有硬编码端口：rdm 组 `18842/18843/18844`（经 stub 内嵌 `http.server` **真实绑口**，同属偶发脆弱类）、若干仅作 svc.env 元数据的 187xx/1889x、以及任务卡已登记的 `tests/test_svc_realcombo.py` 9471/9473/9474。**均不在本任务授权范围**（任务卡非目标明示「不含其它用例的端口治理」，并已把 realcombo 记为遗留发现）；动态临时口（32768+）与它们不相交，本修复既不掩盖也不交互。属后续独立增量，非阻塞。 + +## 重点 5：commit message 与 PR 描述是否如实 + +- transport commit message 仅为传输元信息（`agent-exec transport 850b1455…/5ca3ade8a790`），不含范围声明，无夸大面。 +- implementation commit `5c3a1d4e…` 对象不在本工作区，其 message **未验证**；PR #668 描述在仓外且本单元禁网，**未验证**。 +- 仓内可核的诚实性证据成立：任务卡的范围陈述与实测 diff 完全一致（「单文件单 hunk」「scripts/svc.sh 不改」「TOCTOU 非阻塞、套件级解法属独立增量」「不解决其它用例硬编码端口，仅记录」）。一处文档小疵见 N3。 + +--- + +## 非阻塞建议 + +- **N1 对象拓扑**：packet 字面审查对象（2fd11ef→d0b217d3 的 diff）混入分支落后于 base 的主线内容；忠实的单元 diff 应对 merge-base/R0（a9566a68）或 transport 父提交（409f4749）取。同步记录显示 c766a81/2841cb4 已并入主线且 `tests/test_svc.py` 与 d0b217d3 一致，合并视角无实际风险；仅提示后续以「相对 main 的最终 diff」复核时勿把主线反向内容误记为本单元改动。 +- **N2 TOCTOU 套件级解法**：reservation-close 到 bind 的窗口是既有 5 处夹具共有的固有模式；若未来要求消除，可考虑传递已绑定的 fd/listening socket 或碰撞重试一次。属独立增量（任务卡已记），不建议在本任务扩围。 +- **N3 任务卡计数笔误**：卡称「8 条断言」，实测该用例 `self.assert*` 为 9 条（base 与修复后均 9）；实质主张「一条未删未弱化」经核对为真。仅文档精度问题。 +- **N4 同类脆弱面**：rdm 组真实绑口的 18842/18843/18844 与 realcombo 9471/9473/9474 仍为固定端口；建议另立任务按同一惯例治理。 + +## 已运行的验证命令与结果（均只读；测试 3 次，≤5 次限额内） + +| # | 命令 | 结果 | +|---|---|---| +| 1 | `git cat-file -t 5c3a1d4eb8f3a5c0c0c5be5cd5` | fatal: Not a valid object name（对象不在本工作区） | +| 2 | `git merge-base --is-ancestor`（2fd11ef↔d0b217d3 双向） | 均非祖先；merge-base = a9566a68 | +| 3 | `git diff 409f4749 d0b217d3` 与 `git diff --stat a9566a68 d0b217d3` | 仅 `tests/test_svc.py` 单 hunk（3+/1−）+ `.task` 卡；产品文件零改动 | +| 4 | `git diff d0b217d3 2841cb4 -- tests/test_svc.py` | 空（被测 HEAD 测试代码与 transport commit 一致）；svc.sh 差异仅 db_create 1 行（主线管道治理，本用例不执行） | +| 5 | `PYTHONDONTWRITEBYTECODE=1 python3 -m unittest tests.test_svc.SvcDbRecoveryTests.test_static_fresh_deploy_registers_pidcheck_and_starts` | **OK**（Ran 1, 0.507s） | +| 6 | 同上用例，前台命令内以临时监听器占住 18951 后运行、结束时 kill 并确认释放（`ss -ltn` 前后核对） | 占口确认 LISTEN → **OK**（0.518s）→ 占用者已回收、18951 released | +| 7 | `PYTHONDONTWRITEBYTECODE=1 python3 -m unittest tests.test_svc.SvcDbRecoveryTests`（注：`-k svc_db_recovery` 因大小写不匹配该类名，改以显式类名运行同一系列） | **OK**（Ran 23, 24.607s） | +| 8 | `cat /proc/sys/net/ipv4/ip_local_port_range` | 32768 60999（与全部固定测试端口不相交） | +| 9 | `git grep -n 18951`（排除 .task） | 仅 tests/test_svc.py:753-754（受保护夹具） | +| 10 | base/transport 用例方法 `self.assert*` 计数对比、`register_pg` 函数体、`svc.sh` `static_do_start`/`check_port_free`/`write_static_reg` 通读 | 9=9 断言一致；register_pg 只写 svc.env；static 路径占口即报错退出、探活语义未变 | + +## 判定标准对照 + +- 用例语义被削弱：未发生（断言 9=9，探活守护链完整）→ 非阻塞。 +- TOCTOU 引入可观察的偶发失败：未发生（失效模式为响亮失败，run 2 决定性场景通过）→ 非阻塞。 +- 未按约束改动的内容被改：未发生（svc.sh/register_pg/测试类结构均未动）→ 非阻塞。 +- 补丁外溢到其他用例或脚本：未发生（单文件单 hunk 实测）→ 非阻塞。 +- 结论：**PASS**。implementation commit message 与 PR #668 描述原文未验证（对象不在本地/禁网），已在重点 5 如实标注，不构成阻塞。