fix(ci): the per-attempt timeout was unenforced and the budget omitted two terms

Third final full-range review. Both findings are in the same mechanism, and
together they explain each other.

The per-attempt `timeout` had no `--kill-after`. Plain `timeout` sends TERM and
then waits indefinitely, so a child that blocks TERM runs arbitrarily long
while `timeout` still reports rc=124 — a bound that reads as enforced and is
not. Measured: 12s elapsed under `timeout 3`. dpkg holding a lock is precisely
the case this script anticipates.

And the outer budget omitted two terms that run between attempts: the
`dpkg --configure -a` repair, which had no timeout at all, and the mirrorlist
rotation. Adding them up honestly gave 798s against a 600s phase ceiling — it
did not fit. The fix is not to pick a bound and assert coverage but to shrink
the parameters until the arithmetic closes: 3 attempts of 40s with the repair
bounded at 30s is 508s worst case under a 510s bound, leaving 90s for a cmake
step measured at ~44s.

That number has now been wrong three times, each time looking like real
arithmetic: computed from default parameters and applied to the passed ones,
then missing terms, then finally complete and revealing it never fit. The
comment now enumerates every term so the sum can be checked, and the guide
carries the rule: a comment claiming a bound covers the worst case must be
able to list each item being summed. Docs no longer restate the figures.

Two minor fixes: `nproc` failing left `jobs` empty and `-j"$jobs"` expanded to
a bare `-j`, the exact violation the adjacent comment cites; and a mirrorlist
line with a TAB but a space inside the URI was still rewritten, so the
whitespace check now covers the URI field however the line is separated.

Author: Liam0205

.github/agentic/setup.sh

@@ -283,22 +283,33 @@ setup_golangci() {
 # The path is covered by .gitignore's `pine-cpp/build*/`, as is CMake's
 # FetchContent cache underneath it, so the worktree stays clean.
 setup_cpp() {
-  # Bound apt as a whole, not just per attempt, and size that bound against the
-  # parameters actually passed rather than the script's defaults.
+  # Bound apt as a whole, not just per attempt, and size the bound from every
+  # term in the retry loop rather than from the obvious one.
   #
-  # ci-apt-install.sh runs its retry loop twice, once for update and once for
-  # install. With 3 attempts of 60s plus the 10s and 20s backoffs, one loop is
-  # at most 210s and both are at most 420s, which is what the outer timeout has
-  # to cover. Budgeting for a single loop would let update alone exhaust the
-  # allowance and leave cmake unreached — the opposite of the intent.
+  # ci-apt-install.sh runs its loop twice, for update and for install. Per loop,
+  # worst case, with ATTEMPTS=3 and ATTEMPT_TIMEOUT=40:
   #
-  # Why these numbers: the phase ceiling is 600s, and locally cmake needs 9s to
-  # configure (including cloning ~70 MB of rapidjson + doctest) plus 35s to
-  # build the test target at -j4, so reserving 420s for apt still leaves ample
-  # room on a slower runner. 3 attempts keeps two mirror rotations, which is the
-  # part that addresses the "mirror answers but crawls" mode from #164.
-  timeout --signal=TERM --kill-after=15s 420s \
-    env ATTEMPTS=3 ATTEMPT_TIMEOUT=60 \
+  #   3 attempts x (40s + 10s kill-after)      = 150s
+  #   2 inter-attempt gaps x (30s + 5s dpkg
+  #     repair + ~2s rotate)                   =  74s
+  #   backoff sleeps of 10s and 20s            =  30s
+  #                                             ----
+  #   one loop                                  254s   ->  both loops 508s
+  #
+  # Hence the 510s bound below. Two earlier versions of this comment were wrong
+  # in the same direction, and both times the number looked like arithmetic: the
+  # first computed the worst case from the script's default parameters instead of
+  # the ones passed here, the second counted only attempts and backoffs and left
+  # out the dpkg repair (then unbounded) and the rotation. Every term inside the
+  # loop belongs in the sum, including the ones that only run between attempts.
+  #
+  # The phase ceiling is 600s and cmake needs ~44s locally (9s to configure,
+  # including cloning ~70 MB of rapidjson + doctest, plus 35s to build the test
+  # target at -j4), so 510s for apt still leaves headroom on a slower runner.
+  # 3 attempts is kept deliberately: it preserves two mirror rotations, which is
+  # the part that addresses the "mirror answers but crawls" mode from #164.
+  timeout --signal=TERM --kill-after=15s 510s \
+    env ATTEMPTS=3 ATTEMPT_TIMEOUT=40 \
     bash scripts/ci-apt-install.sh libluajit-5.1-dev libcurl4-openssl-dev || return 1
   # Assert rather than merely print: `set -e` is deliberately off here, so an
   # unguarded `cmake --version` would emit "command not found" and carry on to
@@ -313,8 +324,12 @@ setup_cpp() {
   # Cap the job count. The runner has 4 cores so nproc alone would be fine
   # there, but this script is also runnable locally, where a bare -j is a
   # standing rule violation in this repo (it swap-storms a big dev box).
+  # Validate rather than trust: if nproc is missing or prints something
+  # unexpected, `jobs` would be empty and -j"$jobs" would expand to a bare -j,
+  # committing the very violation this block cites.
   local jobs
-  jobs=$(nproc)
+  jobs=$(nproc 2>/dev/null)
+  [[ "$jobs" == +([0-9]) ]] || jobs=4
   [[ $jobs -gt 12 ]] && jobs=12
   cmake --build pine-cpp/build-tests --target pine_cpp_tests -j"$jobs" || return 1
 }

llmdoc/guides/ci-quality-baseline.md

@@ -89,7 +89,9 @@ Benchmark job 将 `go test -bench` 输出写入 `benchmark.txt`，同时追加
 
 `ci-apt-install.sh` 的结构：update / install 各自最多 3 次尝试（`ATTEMPTS`，默认 3）、每次独立 per-attempt timeout（`ATTEMPT_TIMEOUT`，默认 300s，均可通过环境变量覆盖）、尝试间 backoff（`attempt * 10`s）、kill 后 `dpkg --configure -a` 修复半配置状态、尝试间轮转 mirrorlist 的 `priority:` 顺序（`APT_MIRRORLIST` 可覆盖路径；非 runner 环境读不到该文件时退化为原行为，不报错）、`Acquire::Retries=3`（覆盖单次尝试内的连接中断）+ `DPkg::Lock::Timeout=60`（等待 unattended-upgrades 类锁持有者）。
 
-轮转实现上有三处不显然、改动时不要退回去：**重写 `priority:` 数值而不是只调整行顺序**（文件里的行序不是 apt 遵循的东西，无显式 priority 的镜像排最后）、**轮转量必须按 priority 序而非文件行序计算**（第一版就错在这里：注释正确地写着「`priority:` 才是生效的东西」，紧接着却按行序左移；当行序与 priority 序不一致时，一次轮转可能把首选镜像留在原地，恰好是要消除的失效模式。验证用例全部取自 runner 真实 mirrorlist，而那份文件行序**恰好**等于 priority 序，于是两种实现不可区分、用例全绿——与 #183「fixture 声明顺序恰好已是字典序」同型。判据：**写下「X 才生效、不是 Y」时立刻检查实现按哪个算，并专门造一个 X 与 Y 不一致的用例**，这种用例不会自然出现在现实样本里），以及**按 tab 字段解析**（格式是 URI + TAB + metadata，把整行当字符串重写会用空格连接 URI 与 metadata，apt 会把空格算进 URI；`arch:`/`codename:`/`component:` 必须原样带过，丢掉它们会扩大一个部分镜像被要求提供的文件范围）。调用方若给出更紧的时间预算，应整体包一层 `timeout` 限制 apt 总时长，而不是只压小 `ATTEMPT_TIMEOUT`——`.github/agentic/setup.sh` 的 pine-cpp 阶段就是这么做的（3 次 × 90s 两轮的最坏情况会吃掉整个阶段预算，cmake 根本轮不到）。
+轮转实现上有三处不显然、改动时不要退回去：**重写 `priority:` 数值而不是只调整行顺序**（文件里的行序不是 apt 遵循的东西，无显式 priority 的镜像排最后）、**轮转量必须按 priority 序而非文件行序计算**（第一版就错在这里：注释正确地写着「`priority:` 才是生效的东西」，紧接着却按行序左移；当行序与 priority 序不一致时，一次轮转可能把首选镜像留在原地，恰好是要消除的失效模式。验证用例全部取自 runner 真实 mirrorlist，而那份文件行序**恰好**等于 priority 序，于是两种实现不可区分、用例全绿——与 #183「fixture 声明顺序恰好已是字典序」同型。判据：**写下「X 才生效、不是 Y」时立刻检查实现按哪个算，并专门造一个 X 与 Y 不一致的用例**，这种用例不会自然出现在现实样本里），以及**按 tab 字段解析**（格式是 URI + TAB + metadata，把整行当字符串重写会用空格连接 URI 与 metadata，apt 会把空格算进 URI；`arch:`/`codename:`/`component:` 必须原样带过，丢掉它们会扩大一个部分镜像被要求提供的文件范围）。调用方若给出更紧的时间预算，应整体包一层 `timeout` 限制 apt 总时长，而不是只压小 `ATTEMPT_TIMEOUT`——`.github/agentic/setup.sh` 的 pine-cpp 阶段就是这么做的（具体参数与那一层 `timeout` 的数值以该文件为准，不在此复述）。
+
+**算这个总预算时必须把循环里的每一项都算进去，包括只在尝试之间才跑的那些。** 这个数在审计中连续错了三轮，每次都长得像一份正经算术：第一次拿脚本**默认参数**算出最坏值，却用它论证**实际传入参数**够用；第二次只数了「尝试次数 × 单次超时 + backoff」，漏掉了尝试之间的 `dpkg --configure -a` 修复（当时还没有 `timeout`、完全无界）与 mirrorlist 轮转；第三次才补全。完整项是：`ATTEMPTS × (ATTEMPT_TIMEOUT + kill-after)` + `(ATTEMPTS-1) × (dpkg 修复上限 + 其 kill-after + 轮转耗时)` + 各次 backoff 之和，**再乘 2**（update 与 install 各跑一轮）。判据：**任何「这个上限覆盖最坏情况」的注释，都要能把被求和的每一项列出来**；列不全就说明还没真算过。
 
 包清单纪律：只安装 runner image 真正缺失的包，不重复安装已预装工具（GitHub runner image 预装 cmake / g++ / build-essential，apt 装的同名包版本更旧且 PATH 排序在后，纯粹是死重，只会放大慢镜像暴露面）；install step 之后应对预装工具做版本断言（如 `cmake --version`、`g++ --version`），使 image 变更导致的依赖缺失在 install 阶段就明确报错，而不是在后续编译步骤里表现为莫名错误。新增 workflow 或 job 时禁止绕过 `ci-apt-install.sh` 直接内联 apt 命令。
 

llmdoc/memory/reflections/agentic-setup-script-and-apt-mirror-rotation.md

@@ -47,7 +47,9 @@
 
 ## 四、预算算术要算，不要估
 
-pine-cpp 阶段给 600s。apt 默认 3 次 × 300s，update 与 install 各一轮，最坏 ~1900s——**apt 一个人就能吃掉整个阶段预算，cmake 永远轮不到**。试着压小 `ATTEMPT_TIMEOUT` 解决不了：算下来 2×120 / 3×80 / 2×90 全都留不下构建所需的时间。正确修法是**整体包一层 `timeout` 限制 apt 总时长**（300s），而不是压小单次尝试。
+pine-cpp 阶段给 600s。apt 按默认参数跑，update 与 install 各一轮，最坏值远超这个数——**apt 一个人就能吃掉整个阶段预算，cmake 永远轮不到**。只压小 `ATTEMPT_TIMEOUT` 解决不了，正确修法是**整体包一层 `timeout` 限制 apt 总时长**，再把参数调到「两轮最坏值 ≤ 这层上限」为止。
+
+这个数我算错了三次（见第七·七节），所以现在不在文档里复述具体数值，一律以 `.github/agentic/setup.sh` 里那段把每一项都列出来的注释为准。
 
 构建所需时间是实测的，不是估的：本地 configure 9s（含 clone ~70 MB 的 rapidjson + doctest）、`-j4` 构建测试目标 35s。有了这个数才敢说 300s 留给构建是宽裕的。这与 `reflections/sanitized-fuzz-time-budget-graceful-stop.md` 那条"预算标定要用实测 worst 而非快日均值"同源。
 
@@ -108,6 +110,18 @@ pine-cpp 阶段给 600s。apt 默认 3 次 × 300s，update 与 install 各一
 
 另一个观察：这个函数每一轮都"看起来已经对了"，因为每一版都修掉了上一版被指出的那个具体形状。真正让它收敛的不是修得更细，而是**换了判据的层次**——从"枚举我想到的坏输入"变成"先整体校验合法性、再规定解读进制"。
 
+## 七·七、同一个时间预算算错三次，每次错法不同
+
+apt 外层总预算这一个数，审计连续三轮各查出一次错，而且**每次的错误形式都长得像一份正经算术**——这是它能连续骗过我三次的原因：
+
+1. **拿默认参数算，用来论证实际参数**：算出「默认 3 次 × 300s 两轮 ≈ 1900s」是对的，但我用这个数去论证「300s 够用于我实际传入的 `ATTEMPTS=3 ATTEMPT_TIMEOUT=90`」。而后者两轮最坏是 600s，300s 只覆盖一半。
+2. **漏项**：改成 60s/420s 时，我只数了「尝试次数 × 单次超时 + backoff」，漏掉尝试之间的两项——`dpkg --configure -a` 修复（当时**完全没有 `timeout`**、无界）和 mirrorlist 轮转。
+3. **补全后发现根本装不下**：把所有项算齐是 798s，而阶段上限只有 600s。这时正确做法不是挑一个数字再声称它覆盖，而是**把参数压到算术真的闭合为止**（现为 3 次 × 40s、dpkg 修复限 30s，两轮最坏 508s ≤ 510s 上限，剩 90s 给 cmake 而 cmake 实测只需 ~44s）。
+
+同轮还查出一个与之相扣的缺陷：per-attempt 的 `timeout` **没有 `--kill-after`**。plain `timeout` 发完 TERM 就无限等待，子进程屏蔽 TERM 时能跑到远超上限**却仍然返回 rc=124**——一个看起来被强制、实际没有的上限。实测 `timeout 3` 下跑满 12s 仍报 124。而脚本注释里预期的 dpkg 持锁场景正是这种情况。这也解释了为什么漏项 2 那么致命：无界的修复步骤 + 假的 per-attempt 上限，两者叠加时总预算完全失控。
+
+判据（已写进 `ci-quality-baseline.md`）：**任何「这个上限覆盖最坏情况」的注释，都要能把被求和的每一项逐条列出来**；列不全就说明还没真算过。以及：**只在尝试之间执行的步骤也要有自己的上限并计入总和**。
+
 ## 待办与已知边界
 
 - setup 脚本不覆盖上游的 `validate_codex` / `validate_claude` job（上游有意为之，validator 要在未经审查的候选提交上把关）。

scripts/ci-apt-install.sh

@@ -123,12 +123,14 @@ rotate_mirrorlist() {
   tmp=$(mktemp) || return 1
   awk -F'\t' -v s=1 '
     /^[[:space:]]*(#|$)/ { print; next }
-    # A mirror line must separate URI from metadata with a TAB. If a line has no
-    # TAB yet contains whitespace, the whole thing lands in $1 and we would emit
-    # a URI with a space inside plus a second priority field — a worse file than
-    # we were given. That input is already an invalid mirrorlist, so refuse and
-    # leave the original alone rather than rewriting it into something stranger.
-    NF == 1 && $1 ~ /[[:space:]]/ { exit 1 }
+    # A URI field must not contain whitespace. Two ways it can: a line with no
+    # TAB at all (metadata separated by spaces, so the whole line lands in $1),
+    # or a line that does have a TAB but still has a space inside the URI part.
+    # Either way, rewriting it would emit a URI with an embedded space, plus in
+    # the first case a duplicate priority field — a worse file than we were
+    # given. Such input is already an invalid mirrorlist, so refuse and leave the
+    # original untouched instead of rewriting it into something stranger.
+    $1 ~ /[[:space:]]/ { exit 1 }
     { uri[++n] = $1
       meta[n] = ""
       # Sentinel so mirrors with no explicit priority sort last, as apt does.
@@ -204,14 +206,22 @@ retry() {
   local attempt rc
   for attempt in $(seq 1 "$ATTEMPTS"); do
     echo "==> ${desc} (attempt ${attempt}/${ATTEMPTS}, timeout ${ATTEMPT_TIMEOUT}s)"
-    timeout "$ATTEMPT_TIMEOUT" "$@"
+    # --kill-after matters here, not just decoration: plain `timeout` sends TERM
+    # and then waits forever if the child ignores it, so the attempt can run far
+    # past its limit while still reporting rc=124 — a bound that looks enforced
+    # and is not. dpkg holding a lock is exactly the case this script expects.
+    timeout --signal=TERM --kill-after=10s "$ATTEMPT_TIMEOUT" "$@"
     rc=$?
     [[ $rc -eq 0 ]] && return 0
     echo "    attempt ${attempt} failed (rc=${rc})" >&2
     if [[ "$attempt" -lt "$ATTEMPTS" ]]; then
       # A kill mid-unpack can leave dpkg half-configured; repair before
-      # retrying (no-op in the common kill-mid-download case).
-      sudo dpkg --configure -a >/dev/null 2>&1 || true
+      # retrying (no-op in the common kill-mid-download case). Bounded, because
+      # this runs inside the caller's overall budget and dpkg can block on the
+      # same lock that caused the failure; unbounded here would let a single
+      # repair swallow the time reserved for the actual work.
+      timeout --signal=TERM --kill-after=5s 30s \
+        sudo dpkg --configure -a >/dev/null 2>&1 || true
       # Move to a different mirror for the next attempt. Best-effort: a
       # missing or read-only mirrorlist (any non-runner environment) just
       # means the retry behaves as it did before, so do not fail on it.