test(#823): 给 L1 并发上限闸门补 Docker 回归(含变异见证) - #835
Conversation
审查指出这道闸门没有可复现的回归:仓里搜 QA_L1_MAX_PAR 只有 qa.sh 一处, 提交信息里的人工采样无法从仓库复现,于是下一次 fail-open 的计数/解析回归 会静默恢复无上限运行。 套件跑的是**真的 scripts/qa.sh**,不是逻辑副本:把 docker 换成 PATH 上的桩 (qa.sh 的 dockerrun() 是 bash -c "$*",会解析到桩),真实闸门代码原样执行。 峰值用事件流算最大重叠,不用采样 —— 采样会漏峰值。 四个用例(审查点名的四种): cap=2 生效值 2,峰值 2 上限确实生效 非法值 two 告警,生效值退回 nproc=8 不是静默不限 前导零 08 生效值 8 按十进制,不撞八进制 0 生效值 0,峰值 7 保留「不限」逃生口 对照:cap=2 峰值 2,而不限/8 时峰值 7 —— 断言有分辨力,不是恒真。 写这个套件时它自己抓到我两个 harness bug: 1) 生效值提取用 grep -oE '[0-9]+',先命中了 "L1" 里的 1,四个用例全报 1 —— 判据没在已知输入上校准过。改成只取 `= ` 之后那个数,并用两组已知 输入(8 / 0)校准; 2) 桩对 build 和 run 一视同仁各睡 0.35s,而 build 是同步的,导致 run 之间 几乎不重叠、峰值恒为 1 —— 高上限下断言没有分辨力。改成 build 立即返回、 run 睡 1.2s。 另:容器是 --network none,qa.sh 会跑 npm view 做 registry 快照,真 npm 会 等 DNS 超时而不是快速失败(第一版就这么跑成超时)。npm 一并桩掉,让被测 闸门成为唯一耗时来源。
自查发现:上一版新增了 tests/test823-l1-concurrency-cap/,但全仓没有任何 地方引用它。用阳性对照判实的 —— test823 在自身目录之外命中 0 处,而同类 的 test745 命中 1 处;同时确认被查文件可读、grep 能命中已知串,排除"查询 没生效"。 也就是说我在一条"补上缺失回归"的 PR 里,交付了一个不会被任何东西调用的 套件 —— 正是 #801 那条 P1(gate_is_wired)说的情形,只不过这次在我自己 的产出上。 注册后再验:test823 命中 1 处,与对照 test745 一致。 自包含也验了:注册之后套件会在自己的 L1 列表里看到自己。实测仍 PASS (退出码 0,四个用例全绿,峰值对照依旧有分辨力:cap=2 → 峰值 2, 不限 → 峰值 7),没有递归或自锁。
自查补一刀:上一版建了个没人会跑的门,已注册(
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 37dcbf38bc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if [ "${1:-}" = "build" ]; then exit 0; fi | ||
| printf 'S %s %s\n' "$(date +%s%N)" "$$" >> "$T823_EV" |
There was a problem hiding this comment.
Record events only for actual docker run calls
When qa.sh launches no L1 containers—for example, if every build fails or dockerrun stops resolving this stub—the initial docker info probe still falls through this branch and records one START/END pair. Because the nested QA failure is swallowed with || true and the cap2 assertion accepts a peak of 1, all four cases can report PASS without exercising the concurrency gate. Restrict event recording to the run subcommand and assert that the expected run calls occurred.
Useful? React with 👍 / 👎.
| IFS='|' read -r p eff warned <<< "$(run_case zero 0)" | ||
| say "- 0(不限) 峰值=$p 生效值=$eff 告警=$warned" | ||
| check zero_eff "$eff" 0 "0 保留为「不限」的逃生口" |
There was a problem hiding this comment.
Assert that a zero cap actually permits concurrency
If the 0 = unlimited behavior regresses to a serial or otherwise capped execution while the status message still prints 0, this case passes because it only checks the parsed effective value and never checks the measured peak. That leaves the advertised escape hatch unprotected; compare this case's peak with the capped case (or otherwise require observable overlap) rather than treating the printed configuration as proof of behavior.
Useful? React with 👍 / 👎.
| RUN chown -R node:node /workspace | ||
| USER node | ||
|
|
||
| ENTRYPOINT ["bash", "/workspace/run.sh"] |
There was a problem hiding this comment.
Add the regression suite to an automated workflow
When a later scripts/qa.sh change regresses this gate, no repository workflow executes this entrypoint: .github/workflows/qa.yml only invokes bash scripts/qa.sh --l1, whose L1_TESTS inventory does not contain test823-l1-concurrency-cap, while e2e-docker.yml never builds this Dockerfile. The test therefore cannot catch regressions unless someone remembers to run it manually; add a dedicated CI build/run step rather than inserting it into the inner L1 list, which would recursively invoke itself.
Useful? React with 👍 / 👎.
| ARG SOURCE_COMMIT | ||
| ENV TEST823_SOURCE_COMMIT=${SOURCE_COMMIT} |
There was a problem hiding this comment.
Verify the source commit against the copied test inputs
When this image is built from a dirty checkout or from a revision other than the supplied SOURCE_COMMIT, the container still prints and accepts that SHA because it is merely copied into an environment variable and run.sh checks only its syntax. This can archive a PASS under a commit that does not contain the code actually exercised, defeating the report's reproducibility claim; compare the copied inputs byte-for-byte with the named revision or embed and verify their authoritative hashes.
AGENTS.md reference: AGENTS.md:L25-L25
Useful? React with 👍 / 👎.
① 桩原来对任何非 build 的 docker 子命令都记事件,峰值会被无关调用抬高。 改成只有 `docker run` 才记。 ② 0(不限)原来只断言生效值为 0 —— 那只证明它被这么解析,没证明它真的 放开了并发。补一条:不限时峰值必须明显高于 cap=2 的峰值。 ③ SOURCE_COMMIT 原来只验 40 位十六进制格式。任何 SHA 都能通过,而报告 里那个 SHA 可能根本不含镜像里被测的文件 —— 这正是我自己在 #801 上 提的那条 P1,建这个套件时原样犯了一遍。 改成:构建时把 run.sh 在该 commit 下的 git blob 哈希作为 build-arg 传入,容器内就地重算并比对(blob 哈希 = sha1("blob <len>\\0"+内容), 不需要容器里装 git)。 第四条「接进自动 workflow」上一提交已自查修掉(注册进 L1_TESTS), 审查针对的是修之前的坐标。
四条 P2:三条已修,一条上一提交已自查修掉
① 桩记多了会污染峰值 —— 修完数字明显更干净原来对任何非 这不是修辞:修前 ② 「不限」原来只验了解析,没验行为原来只断言 ③ SHA 绑到字节 —— 这条正是我自己在 #801 上提的那条 P1,我建这个套件时原样犯了一遍原来只验 改法:构建时把 见证红两种,都真跑过: 第二种是关键:它模拟的正是「SHA 与被测字节脱钩」这个真实病症,而不只是参数写错。(篡改后已还原, 这条 PR 到现在,四次里有两次是我犯了自己刚指出的那一类
同一条 PR 里我提出的两条判据,两条都在我下一件产出上失效了。说明"提出过一条判据"和"它对我自己生效"之间,还差一个机械步骤 —— 前者是我做的,后者目前靠审查补。 |
审查(通信狗 FINAL MAJOR)指出 §1 自相矛盾:一边说报告作为 report-only 子提交落在 SRC 之上,一边自查却要求锚点「等于 git rev-parse HEAD」。 报告提交后 HEAD 合法地高于 SRC,stack 分支同理 —— 这条规则会把正确的 做法系统性判成假红。 这条尤其该修:我自己在 #835 上用的正是 report-only 子提交模式。 按第一版的规则,我的清单会否掉我自己的正确做法。 改成正确的不变量: 锚点 == SRC(建镜像时传进去的 build-arg,即被跑的那份字节) 且 git merge-base --is-ancestor "$SRC" HEAD 同时把最后一行判据的强度说清楚:git show SRC:<文件> | grep -c 只证明 「那一版的代码能产出这种输出」,是必要非充分 —— 不证明这一次的输出 就来自它。要证 provenance 得靠镜像与字节的绑定(例如把被测文件在该 commit 下的 blob 哈希传进容器里重算比对),而不是靠字符串出现过。 另修一处错链:§3「它自己通常不会检查这些要求(见第 8 条)」—— 第 8 条讲的是扫描器校准,该指的是「失效四:门的结论建立在一个它 从不检查的前提上」。已改。
⑥ SOURCE_COMMIT 只验格式不验字节
原来只验 ^[0-9a-f]{40}$。任何 SHA 都能过,而审查指出提交进来的 report
里那个 SHA 早于本套件自身 —— 那份证据无法从它自称的版本复现。
改成与 test823 相同的做法:构建时把 run.sh 在该 commit 下的 git blob
哈希作为 build-arg 传入,容器内就地重算比对(blob 哈希 =
sha1("blob <len>\0"+内容),不需要容器里装 git)。
已验脚本内算法与 git hash-object 结果一致;该机制的端到端红/绿在 #835
上证过两次(传错 blob、blob 对但文件被篡改,都 exit 1)。
⑤ qa.yml 缺 test601 路径
test798 的镜像 COPY 了 test601 的 race-worker.ts,而
server/src/scheduled-tasks-http.test.ts 会执行它做「两个真 Hub 抢同一
occurrence」。只改那个 worker 的 PR 不该跳过这道门。已在两处 paths 补上。
(这 4 行原本只存在于 #798;若只合本 PR、把 #798 当冗余关掉,它们永远
不会落地 —— 此前已在本 PR 记录过这个坑。)
② server 的 npm install 无 lockfile —— 我没有改,需要所有者决定
实测:server/package.json 有 4 个依赖,4 个全用 caret 范围,且仓里没有
任何 lockfile/shrinkwrap。所以同一个 commit 在不同时间构建确实会解析出
不同依赖图,审查这条成立。
但修法只有一条:提交一份 lockfile。那是仓库级的依赖钉死决策 —— 它影响
每一次 server 构建,不只是这道门;而且生成出来的树我无法在这里验证是否
仍然全绿。这不该由我单方面决定,如实留作待决,不假装已修。
本 PR 的 L0+L1 稳定红,失败行只有一句: FAIL: TEST823_SOURCE_COMMIT 必须是一个完整的小写 SHA(收到 '') 根因不在被测的门,在供给侧。qa.sh 里原本是一串逐套件的 elif: if [[ "$t" == "test686-rest-shape-golden" ]]; then --build-arg TEST686_SOURCE_COMMIT=… elif [[ "$t" == "test765-batch-runtime-gate" ]]; then … elif [[ "$t" == "test766-bunx-preflight" ]]; then … elif [[ "$t" == "test746-setup-bun-pin" ]]; then … fi 本 PR 把 test823 加进了 L1_TESTS,但没人记得这里也要加一条 —— 于是 TEST823_SOURCE_COMMIT 是空串,门正确地 fail-closed。 只补一条 elif 能让它变绿,但下一个新套件还会踩同一个坑: 「注册了套件」和「在供给侧登记」是两处,分开就会漂。所以改成按名推导: testNNN-... → --build-arg TESTNNN_SOURCE_COMMIT=$(git rev-parse HEAD) qa-*-... → 不传(与原行为一致,它们的门不要这个变量) 行为等价性验证(对当前 L1_TESTS 全部 18 个套件逐个模拟): test823-l1-concurrency-cap → TEST823_SOURCE_COMMIT (新增,本 PR 需要的) test686-rest-shape-golden → TEST686_SOURCE_COMMIT (与原 elif 一致) test765-batch-runtime-gate → TEST765_SOURCE_COMMIT (一致) test766-bunx-preflight → TEST766_SOURCE_COMMIT (一致) test746-setup-bun-pin → TEST746_SOURCE_COMMIT (一致) qa-cli-01 / qa-hub-05 / qa-node-03b / … → 不传 (一致) bash -n 退出码 0。 顺带记一条同类:#801 的红是同一个形状 —— run.sh 要求 TEST798_RUNSH_BLOB、 Dockerfile 接了线、workflow 的 docker build 从没传。都是「门要求 X, 供给侧不知道要给 X」。
上一个提交(2bb734a)把逐套件 elif 改成按名推导 TESTNNN_SOURCE_COMMIT。 方向对,但**覆盖不全**:CI 照旧红在同一行 FAIL: TEST823_SOURCE_COMMIT 必须是一个完整的小写 SHA(收到 '') 原因是仓里并存两套命名,而我只按其中一套推导: tests/test686-rest-shape-golden/Dockerfile ARG TEST686_SOURCE_COMMIT tests/test765-batch-runtime-gate/Dockerfile ARG TEST765_SOURCE_COMMIT tests/test766-bunx-preflight/Dockerfile ARG TEST766_SOURCE_COMMIT tests/test746-setup-bun-pin/Dockerfile ARG TEST746_SOURCE_COMMIT tests/test823-l1-concurrency-cap/Dockerfile ARG SOURCE_COMMIT / ARG RUNSH_BLOB ← 不一样 test823 的 Dockerfile 收的是 `SOURCE_COMMIT`,再由它自己组装 `ENV TEST823_SOURCE_COMMIT=${SOURCE_COMMIT}`。我传的是 TEST823_SOURCE_COMMIT, 名字对不上 → ARG 空 → ENV 空 → 门 fail-closed。它还要 RUNSH_BLOB(run.sh:28)。 这次两套都传。未被 Dockerfile 声明的 build-arg 只产生一条警告,不影响构建。 blob 等价性实测(本分支 head 上): git rev-parse HEAD:tests/test823-l1-concurrency-cap/run.sh { printf 'blob %d\0' "$(wc -c < run.sh)"; cat run.sh; } | sha1sum 两者相同 —— 与 run.sh:31 的算法一致。 bash -n 退出码 0;对 L1_TESTS 里各形态逐个模拟,qa-* 仍不传。 🔴 记一条:上一版我验证了「四个旧套件行为逐条复现」,那个验证是对的, 但它只覆盖了我知道的那套约定 —— **我没有去核每个 Dockerfile 实际声明了什么 ARG**。 「与原行为一致」不等于「对所有套件都正确」。
第三次 CI 仍红,但**换了一种红法**,而且这次是我造成的。 前两次红的是 `TEST823_SOURCE_COMMIT 收到 ''`。那个已经修好了 —— 本次日志里 `source_commit=1f2ab57a…` 正常出现、blob 校验也过了。 这次红在: - cap=2 峰值=0 生效值=2 告警=0 FAIL cap2 - 0(不限) 峰值=0 生效值=0 告警=0 FAIL zero_conc failures=2 **每个用例的峰值都是 0** —— 桩一次都没被调用。根因: scripts/qa.sh:17 set -euo pipefail test823 的 Dockerfile 只装 bash / ca-certificates / coreutils / procps —— **没有 git** test823 的 run.sh 桩了 docker 和 npm,**没有桩 git** 而我上一版把 `$(git rev-parse HEAD)` 从「4 个具名套件」扩到了「所有 testNNN 套件」。 于是在 test823 重放 qa.sh 的那个容器里:git 不存在 → 127 → set -e 当场中断 → docker 桩一次没被调用 → 事件流为空 → 峰值恒 0 → 闸门自己的回归失败。 **这不是被测代码的问题,是我改出来的回归。** 修法:git 调用全部 `2>/dev/null || true`,取不到就不拼 build_args —— 无 git 环境下退回到「和我动手之前一样」的行为(不传 build-arg), 真 CI 里 git 在,照常传。 模拟验证(PATH 置空以制造无 git 环境,带 set -euo pipefail): 未中断,build_args 为空。bash -n 退出码 0。 🔴 教训:我改的是**一个会被别的门重放的脚本**。给它加依赖(git)时, 我只想着「CI runner 上当然有 git」,没想过它还会在一个刻意最小化的容器里被重放。 「这个环境肯定有 X」——当脚本本身是被测对象时,这句话要先证明。
修 #823 的 P1(
scripts/qa.sh:191「Add a persisted Docker regression for the gate」)。审查说得对:仓里搜
QA_L1_MAX_PAR只有qa.sh一处,提交信息里的人工采样无法从仓库复现,所以下一次 fail-open 的计数/解析回归会静默恢复无上限运行。套件跑的是真的
qa.sh,不是逻辑副本把
docker换成 PATH 上的桩 ——qa.sh的dockerrun()是bash -c "$*",会解析到桩,真实闸门代码原样执行。在副本上测只能证明副本自洽。峰值用事件流算最大重叠(每次桩调用写精确的 START/END 纳秒时间戳,事后排序),不用采样 —— 采样会漏峰值。
四个用例(审查点名的四种)
cap=2two080对照有分辨力:
cap=2时峰值 2,而不限/8 时峰值 7 —— 不是恒真断言。变异见证(没见过红的门不算门)
去掉
qa.sh里的取值校验段(字节改动已验,跑完还原到原 hash),套件变红,且理由都对:report 的 SHA 刻意钉在含套件的那个提交上
Source: 08f54e8b是包含被测套件本身的提交,不是它的父提交。#801 上有一条 P1 正是「report 里的 SHA 早于套件本身,证据无法从其标注的版本复现」——这里先提交套件、再按该 SHA 建镜像跑、最后把结果作为 report-only 子提交落下。写这个套件时它抓到我两个 harness bug
grep -oE '[0-9]+',先命中了 "L1" 里的那个 1,四个用例全报 1 —— 判据没在已知输入上校准过。改成只取=之后那个数,并用两组已知输入(8/0)校准;build和run一视同仁各睡 0.35s,而 build 是同步的,导致 run 之间几乎不重叠、峰值恒为 1,高上限下断言没有分辨力。改成 build 立即返回、run 睡 1.2s。两个都是「门看起来在跑、其实测不出东西」那一类 —— 是这个套件自己红出来的,不是我看出来的。
容器
--network none(npm view也桩掉,否则真 npm 会等 DNS 超时,第一版就这么跑成超时)。测试镜像用完即删,未动任何他人镜像。