Conversation
Signed-off-by: shangzh0 <97216392+shangzh0@users.noreply.github.com>
Signed-off-by: shangzh0 <97216392+shangzh0@users.noreply.github.com>
huangruiteng
left a comment
huangruiteng
left a comment
There was a problem hiding this comment.
动机
可见 CLI 循环的宿主机连接器 scripts/external_scheduler_worker.py 在收到"该停了"的调度包时会进入错误的循环：它在解析节奏之前就要求 scheduler_hint.cold_path_detail.local_scheduler 存在，而控制面构造停机包的那条路径（loopx/control_plane/scheduler/scheduler_hint.py:217 的 _build_scheduler_stop_hint）本来就不会写这个字段。结果就是 parse_tick 抛 ValueError，run_worker(:329) 记一行 status=tick_error 然后睡 error backoff 再轮询——一个已经终止或对等协调被阻塞的 Goal，宿主机定时器永远停不下来。这个 PR 加了 LHTB 这条实验臂（Harbor + LoopX generic_cli 心跳 + 每次唤醒一条全新 codex exec），并顺带修掉这个上游缺陷。
诉求成立，而且修法方向是对的：让 worker 先判断"这个包是停机"，再去读永远不会被用到的节奏数据，比让生产者伪造一份节奏数据小得多，也没有第二份权威。影响面是运维者与这条实验臂的维护者，没有终端用户面；不做这件事的代价是一个合法的停机状态会持续产生误导性的错误状态行。真正需要再收一步的是"停机"这个词表本身：生产者用同一个函数发出两个停机动作，而实现只认了一个。
改动思路
入口是 benchmark/LHTB/run.sh（preflight / smoke / full）→ configs/heartbeat-generic-cli.yaml 的 import_path: codex_loopx_heartbeat:LoopxHeartbeatCodex → 每个 trial 容器内的适配器 → scripts/external_scheduler_worker.py（shell_worker 连接器）→ 允许唤醒时调用 --wake-cmd runtime/wake_once.py。权威输入是 quota should-run --runtime-profile generic_cli --include-detail scheduler 的 JSON，加上 worker 本地那份很小的状态文件（reset_token、unchanged_count）。决策所有者是控制面：SchedulerDisposition.TERMINAL_STOP 与 PEER_COORDINATION_STOP 都经 _build_scheduler_stop_hint 落成 hint，同时给出 unchanged_poll.local_scheduler = "stop"；worker 只是投影者，它要回答的只有一个问题——还能不能再唤醒一次。
正路径清晰：parse_tick(:92) 看到 terminal 动作就返回 terminal=True，run_worker(:349) 打印 status=terminal action=... class=...、把 {reset_token, unchanged_count: 0} 落盘并 exit 0，不再 sleep、不再唤醒。非 terminal 的包仍然走原来的节奏阶梯（select_interval:236），缺 cold_path_detail 时依旧 fail closed——这条基线行为我用同一次探针验证过，没有被放宽。
与既有实现的关系有两处需要分开看。第一处是既有的持久化冒烟 examples/external-scheduler-worker-smoke.py：它是这条 worker 契约的公共守卫，但它的 _hint_payload 无条件构造 cold_path_detail，两个 terminal 用例（:116、:315）都带着节奏数据，所以本次新契约的覆盖面只落在新增的单测上。第二处是 benchmark/swe-marathon/agents/codex_offline.py：新臂没有复用它，而是新写了一个同名同义的 CodexOffline。worker 这边"短路放在消费者"是正确归属；adapter 这边则是把已有的适配器契约又拥有了一遍。
具体改动
21 个文件、+2134/−5。上游只有两个文件：scripts/external_scheduler_worker.py +21/−5（唯一的行为改动）、tests/test_external_scheduler_worker.py +26。其余 19 个文件全是 benchmark/LHTB/ 下的新内容：README 206、run.sh 213、agents/codex_loopx_heartbeat.py 579、runtime/wake_once.py 289、scripts/preflight.py 239、harbor_patch/prepare_harbor_modelonly.py 131、agents/codex_offline.py 122、配置 76、render/summarize 62/51、两个 verifier Dockerfile、.env.example、.gitignore、两个 .gitkeep。
关键代码讲解
parse_tick（scripts/external_scheduler_worker.py:92）：改动后先算 action，命中 TERMINAL_ACTIONS 就直接返回 terminal=True, interval_minutes=1, progression=(1,), unchanged_limit=None, after_limit="stop_tick_loop"(:113)，把 _extract_local_scheduler 推到后面；节奏路径则硬编码 terminal=False(:160)。这 21 行就是"停机包不再需要节奏数据"的全部实现，方向正确。
TERMINAL_ACTIONS（scripts/external_scheduler_worker.py:45）：内容没变（frozenset({"stop_until_explicit_resume"})），但角色从"事后打标"变成"第一道准入"。它现在实际定义的是 worker 认可的停机词表，而这个词表比生产者能发出的停机动作窄——这就是本次阻断项的落点。
run_worker 的 terminal 分支（scripts/external_scheduler_worker.py:349）：分支体没改，但第一次对缺 cold_path_detail 的包可达。它打印运维者等待的那行 status=terminal ...、用 _save_state 原子写状态（0600）并返回 0；:329 的 except (RuntimeError, ValueError) 则把解析失败折成 status=tick_error + 退避轮询，两条路径的可见后果差别很大。
main（benchmark/LHTB/runtime/wake_once.py:146）：一次唤醒的全部契约——mint 唯一 turn_id、要求 heartbeat-prompt 回包 ok=true 且 turn_instance_id 回环一致、task_body 非空且带 --runtime-profile generic_cli、把全局 registry token 替换成本地路径，然后以每轮独立的 CODEX_HOME 起一条全新 codex exec（fresh_codex_exec: true、resume: false），最后写 receipt.json 并删掉临时目录。这些前置校验都在任何模型调用之前，缺一项就抛错，符合"每次唤醒独立"的臂定义。
CodexOffline.install（benchmark/LHTB/agents/codex_offline.py:70）：把原生 Codex 包（codex、code-mode sidecar、rg、bwrap）从宿主机塞进容器，让 22 个 allow_internet=false 的任务不必联网安装。功能与 benchmark/swe-marathon/agents/codex_offline.py:71 的同名方法等价，差别只在注释、报错文案与 _STAGE_DIR 的取法——这正是 P3 说的重复。
对主干的风险
P2（阻断）——停机词表只覆盖了生产者两个停机动作中的一个。 控制面在 PEER_COORDINATION_STOP 时同样走 _build_scheduler_stop_hint，产出的包是 action="return_to_owner_until_material_change"、unchanged_poll.local_scheduler="stop"，且同样不含 cold_path_detail（scheduler_hint.py:1316-1335）。我在 exact head 用真实模块复现：同一个 parse_tick，stop_until_explicit_resume 缺节奏数据 → terminal=True；return_to_owner_until_material_change 缺节奏数据 → 仍抛 ValueError: scheduler_hint.cold_path_detail.local_scheduler missing，于是回到 status=tick_error + 退避的无限轮询。也就是说，PR 描述和 benchmark/LHTB/README.md 里"terminal scheduler packets 可以省略 cold_path_detail"这句话对第二个动作不成立，而对等协调被阻塞恰恰是最需要停下来的状态之一。最小修法是把判定改成复用生产者的停机指令（例如 scheduler_hint.unchanged_poll.local_scheduler 以 stop 开头）而不是维护字面集合；若坚持列集合，则必须补齐每个停机动作。回归建议：按动作做一张 parse_tick 表，各自断言 terminal is True、after_limit == "stop_tick_loop"，并断言 run_worker 退出 0。
P3（非阻断）——这次契约的落地没有进公共冒烟，新单测锁的也不是不变量。 examples/external-scheduler-worker-smoke.py 的 terminal 用例仍带 cold_path_detail，test_missing_detail_fails_closed 用的是非 terminal 动作，所以"停机包缺细节"只有新单测覆盖；而新单测只断言 run_worker == 0 与状态文件内容，不断言 terminal/after_limit，一旦这条分支回退，它会沿着 5 秒退避一直转（表现为挂住）而不是失败。同一个契约在 benchmark/LHTB/scripts/preflight.py:113-118 里的"terminal scheduler compatibility"检查也只是比较两行源码的先后顺序，词表不完整时照样通过。建议把断言直接落到决策形状，并给公共冒烟补一个缺细节的 terminal 用例。
P3（非阻断）——新臂复制了一份已有的离线 Codex 适配器。 benchmark/LHTB/agents/codex_offline.py（122 行）与 benchmark/swe-marathon/agents/codex_offline.py（204 行）同名、同基类、同安装契约，diff -u 后只在注释、错误文案和 _STAGE_DIR 取法上有差异；老的副本里还留着几条踩坑结论（web_search 的正确键名、harbor provider 必须有 name、code-mode sidecar 不能漏、限流与重试的真实原因），新副本把这些都删掉了。后果是下一次适配器修复要改两处，且其中一处已经失去理由记录。建议改为 import 既有适配器，或把唯一一份挪到两个臂都能用的位置；确需分叉时把差异理由写在差异旁边。
残余风险与证据边界。 我实际跑过的是：exact head 上加载真实 worker 模块做的 3 动作 × {有/无 cold detail} 探针、pytest tests/test_external_scheduler_worker.py -q（5 passed）、loopx canary premerge --from-git-diff（全部通过，0 失败，仅 1 条 benchmark-sensitive 人工 hold，公共/私有边界扫描覆盖 21 个改动路径）、两份 adapter 的 diff -u，以及停机包生产路径的源码核对。没有跑的是 LHTB 端到端 trial（需要 LHTB checkout、Harbor 虚拟环境、Docker 与模型网关），所以这条臂的"能否跑起来"只有契约与代码路径层面的证据。分支落后于 origin/main（合并基 f22879d50，git merge-tree 干净），任何 rebase 之后本次复现都要重跑；按本轮解析到的策略（wait_for_ci=false）没有取用远程 CI，也不把远程 CI 当作证据缺口。
我的整体评价
我认可这个方向的判断和大部分实现：上游那 21 行把"停机包先判停机"做对了，非 terminal 包的 fail-closed 语义原样保留，新臂是一条自洽、可关闭的研究资产（只有 run.sh 主动启用，Harbor 网络补丁由 LHTB_MODELONLY_NET 控制、缺省仍是 network_mode:none，21 个改动路径的公共/私有边界扫描干净，.env.example 用的是 RFC 5737 文档网段，.gitignore 把 runs/、reports/、.generated/、.env 都挡在外面）。runtime/wake_once.py 一次唤醒一套独立 CODEX_HOME、每次全新 codex exec、身份回环校验与 receipt.json 也都在正确的边界上。
阻断我的是一个"改一半"的不一致：同一个生产者函数发出的两个停机动作，消费者只认了一个，于是另一个仍然退回到本次修改要消灭的 tick_error 循环；而 PR 描述与 README 用的却是"terminal packets"这个更宽的说法。这个修法很小（把判定改成从生产者的停机指令派生，或者补齐动作并逐个加用例），修完这次改动就只剩两条 P3：把契约落进公共冒烟并把断言收窄到不变量，以及消掉重复的离线 Codex 适配器。请从新的 exact head 重新开始评审。
English verdict: REQUEST_CHANGES — exact head f33a6f4c5b129886bf99bd2f48b74c09daaab126 of #4464. The direction is right and verified: at this head a stop_until_explicit_resume packet without cold_path_detail.local_scheduler returns terminal=True, after_limit=stop_tick_loop, run_worker exits 0 with the unchanged {reset_token, unchanged_count: 0} state, non-terminal packets still fail closed without cadence detail, pytest tests/test_external_scheduler_worker.py -q passes 5/5, loopx canary premerge --from-git-diff reports 0 failures (one benchmark-sensitive manual hold) and the public/private scan over all 21 changed paths is clean. One blocking P2: TERMINAL_ACTIONS covers only one of the producer's two stop actions — _build_scheduler_stop_hint (loopx/control_plane/scheduler/scheduler_hint.py:217 and :1316-1335) also emits return_to_owner_until_material_change with unchanged_poll.local_scheduler="stop" and no cold-path detail, and the same in-process probe shows it still raises ValueError: scheduler_hint.cold_path_detail.local_scheduler missing, so run_worker (:329) keeps logging status=tick_error and repolling instead of stopping, contradicting the PR's own "terminal scheduler packets" claim; minimum repair is to derive terminality from the producer's stop directive (for example scheduler_hint.unchanged_poll.local_scheduler) or cover every emitted stop action, one parse_tick case per action. Two non-blocking P3s: the durable smoke examples/external-scheduler-worker-smoke.py still builds every terminal fixture with cold_path_detail and the new unit test asserts only exit code plus state file (so the regression path would hang on the 5s backoff rather than fail, and benchmark/LHTB/scripts/preflight.py:113-118 only compares source line order); and benchmark/LHTB/agents/codex_offline.py re-declares an already-diverging copy of benchmark/swe-marathon/agents/codex_offline.py, dropping the recorded failure knowledge. Residual risk: no LHTB trial was executed here (needs an LHTB checkout, Harbor, Docker and the model gateway), remote CI was intentionally not consulted (wait_for_ci=false), and the branch is BEHIND main, so all reproductions must be repeated after any rebase.
|
Superseded by #4504 from a fresh current-main branch. The new PR addresses the owner review by deriving terminality from |
2 participants
Summary
benchmark/LHTBgeneric_cliheartbeats through one freshcodex exec --jsonper wake, with nocodex exec resumereplan_after_completed_todos=3Validation
python3 -m pytest -q tests/test_external_scheduler_worker.py(5 passed)ruff check benchmark/LHTB scripts/external_scheduler_worker.py tests/test_external_scheduler_worker.pypython3 -m compileall -q benchmark/LHTB scripts/external_scheduler_worker.py tests/test_external_scheduler_worker.pybash -n benchmark/LHTB/run.sh benchmark/LHTB/scripts/build_verifier_images.shbenchmark/LHTB/run.sh preflightagainst an LHTB checkout: 46/46 tasks, 22 offline/24 online, 44 shared/2 separate verifier tasks, fresh-exec contract, Web Search disabled, and replan threshold readback all passedloopx canary premerge --from-git-diff --git-diff-base origin/main --tier standard: all direct checks, 10 catalog canaries, 8 risk-profile smokes, and the public-boundary scan passed; the expectedbenchmark_sensitivemanual-review hold prevents self-mergeBoundaries
Scope review
The runner keeps benchmark-specific lifecycle code under
benchmark/LHTBand reuses the existing external scheduler contract. The only core runtime change is the narrow terminal-packet parser fix covered by its regression test. Public configuration uses explicit external checkout/runtime paths rather than machine-specific layout assumptions.