test(host): publish process markers atomically - #5365
huangruiteng merged 2 commits into
Conversation
Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
|
CI attribution for exact head
Both failures are the concurrent runtime-source fingerprint race already reproduced on current main. They are unrelated to this PR's atomic Host marker change. The dedicated baseline repair is #5367 at exact head |
|
Follow-up after |
huangruiteng
left a comment
There was a problem hiding this comment.
评审精确 head:5a20d3da8f3c89d61ca42b7ccf63dfc924904d5e,不可变基线:67930ab6af78491f10ca3de4ff74ef7a39954a51。按当前 LoopX PR review capability(policy revision 12)完成全量 diff、真实进程验证及反例检查。没有阻塞发现,结论 APPROVE;本评审不执行合并。
动机
这条 PR 解决的是终止测试的观测竞争,不是修改 Host 的终止规则。旧 fixture 用写入模式直接打开已发布的 counter:打开先截断、写入随后发生,子进程若恰在两者之间被清理,最终 marker 会变为空字符串。测试便把“完整旧值 → 空内容”误判成子进程仍在工作,给正常 cleanup 报假失败。
我独立强制暂停在截断/写入窗口再杀死进程:旧式直接发布留下空内容,同目录 staged publication 保留完整旧值。新回归也能确定性拒绝直接发布 mutant。因此这不是只固定现有输出的测试增量,而是关闭一个可复现的测试可靠性缺口;命名任务已经完成,不代表关闭更大的 review 或 runtime 项目。
改动思路
发布内容先写到 marker 同目录、带 PID 的临时文件,再用 os.replace 替换公开 marker。读取者只能观察已发布的完整 counter;进程在 staged write 或 replace 前被杀死时,旧 marker 保持完整。这里需要的是进程中断下的原子可见性,不是断电持久性,因此不增加 fsync、生产重试或新的 lifecycle owner。
通用 Host 与 Codex fixture 共享同一份 child source,仍使用当前解释器、忽略 SIGTERM、持续递增,供真实 TypeScript supervisor 的 SIGKILL 兜底路径处理。生产链仍是 Python transport → host_process_bridge.ts → runHostProcess → group cleanup → 原有结果消费者;本 PR 不改该链的源码、deadline、grace period 或结果合同。
具体改动
三个文件合计 +94/-28,全部属于测试或 fixture,没有生产、依赖、生成资产或文档改动。
关键代码讲解
tests/control_plane/host_process_fixture.py::COUNTER_PROCESS_SOURCE:28 行共享 source。子进程自行记录 PID,构造同目录 staged 文件,先写完整 counter 再替换 marker;可选 pause fence 只供确定性中断测试调用。tests/control_plane/test_host_process.py::test_counter_process_fixture_publishes_atomically:预置已发布值,等待明确 fence,kill 并 wait 后检查旧值未损坏;finally 确保自有进程退出。owner 消失和 Host timeout 两条原有真实进程测试也改用共享 fixture,原有 cleanup 断言保留。tests/test_loopx_turn_codex_cli.py::_fake_codex:用 repr 注入共享 source,并通过 argv 传入 marker、PID path 与 interval,而不是在 child source 中插入路径。result 和 timeout 两条 descendant 测试仍走真实 adapter/supervisor。PID 改由 child 写入,且在首次 marker 发布之前完成,保持 cleanup 的身份对应关系。
独立正向检查还证明:无 pause 时 counter 持续增长;SIGTERM 被忽略后仍增长;SIGKILL 后稳定;PID 文件等于真实 child PID。这补足了“暂停路径通过,但正常 writer 根本不工作”的反例。
对主干的风险
最强回归风险是测试因观测竞争报假失败,或者 fixture 不再工作而令 liveness 断言假通过。完整模块与独立正/负控制都已覆盖;产品终止行为源码在 base/head 逐路径相同。新增临时文件只在测试临时目录内,kill 后可能留下 staged 文件,由临时目录管理,不新增持久产品状态。
本次本地验证:
uv run --extra test python -m pytest tests/control_plane/test_host_process.py tests/test_loopx_turn_codex_cli.py -q:head 48 passed;同命令 base 47 passed。node --no-warnings --experimental-strip-types --test tests/control_plane_ts/host_process.test.ts:9 passed,覆盖 timeout、abort、leader exit、closed pipes 与 callback failure 的真实进程清理。- 独立 marker interruption/mutation controls:原生 fence 保留旧值、staged truncate gap 保留旧值、旧式 truncate gap 为空、原生回归拒绝直接发布 mutant、正常运行与信号检查全部满足预先定义的不变量。
- 三个变更文件的 Ruff、compile、公开边界扫描与
git diff --check通过;npm run -s typecheck:control-plane、仓库配置的 mypy(19 source files)通过。 - 精确 diff 的 change-quality receipt 已记录并 verify 为 valid;
uv run --extra test loopx canary premerge --from-git-diff --git-diff-base 67930ab6af78491f10ca3de4ff74ef7a39954a51 --goal-id GOAL通过 4 个直接检查。Planner 对这个 tests-only diff 没有选择 catalog smoke;以上 48 Python/9 TS 项提供实际行为覆盖,不把“未选择”写成“已运行”。
基线失败归因
我读过作者的失败说明,但没有以作者声明替代证据。对 tests/control_plane/test_turn_journal_runtime_readiness.py 的以下三个测试,在上述不可变 base 和 exact head 用同一 uv run --extra test python -m pytest ... -q -k selector 独立运行,均为 3 failed/14 deselected,失败身份及细节一致:
test_runtime_fingerprint_rescans_when_a_snapshotted_file_disappears_while_reading:缺少期望的第二次 first.ts 读取;test_runtime_source_churn_has_a_stable_readiness_diagnostic:ready 而非 package_invalid;test_runtime_request_source_churn_raises_a_stable_startup_diagnostic:runtime_exited_before_ready 而非 packaged_runtime_source_unstable。
相关 effect_runtime 与 readiness 测试源码在 base/head 未变,marker 不参与其 fingerprint/startup 决策,改变的不变量有独立通过证据,故分类为 pre_existing_unrelated,不要求这条 tests-only PR 修复无关 runtime owner。恢复由独立 #5367 处理;我确认其 merged 状态,但没有把其他 head 的验证挪用到 #5365。当前策略 wait_for_ci=false,本次不查询/等待 CI;APPROVE 不是 merge-readiness 结论。
残余限制:在 macOS/POSIX 的真实进程上验证,没有声明原生 Windows 或所有 Python 版本覆盖;本次也未运行全量 pytest/paid model。新增测试显式限定 POSIX,不放宽原有跨平台规则或 required check。
我的整体评价
这是完整、可逆、范围合适的测试维护修复。三个真实消费者一起消除同一竞争,且有确定性 regression 和 mutation pressure;无需生产改动、另设 capability/provider 或扩大到 runtime fingerprint 修复。
已完成相邻边界的 future-facing pass:PR 本身去掉重复 child source、保留各入口独立语义断言,没有必要再抽一层 framework。现有覆盖搜索和作者最近 25 条 PR 的 scope/时间检查未发现这条 atomic-marker 修复与其他 PR 同形重复;其它 counter 测试保护 scheduler/delegation 合同,不应因相似名字被合并。long_horizon 产品执行语义保持不变,贡献者验证体验因减少假失败而改善;前端、Lark 与普通 CLI 交互均未变,不需要 companion UI 工作。
typed-state、domain-neutrality、default-off isolation、authority naming、行为披露及 guidance-versus-obligation 六个镜头已核对:这里只更换测试观测机制,没有新增或改变共享状态词表、激活 gate、默认产品行为、机器义务、权限或持久 receipt 合同。没有阻塞发现;后续若 head 更新需重做 exact-head 验证,合并仍须独立 readiness 与授权。
English verdict: APPROVE - HEAD 5a20d3d. The tests-only atomic marker fix removes a reproduced truncate/write race without changing Host termination semantics. Exact-head Python 48/48 and TS Host 9/9 passed; deterministic interruption/mutation controls, lint, types and exact-scope canary passed. Three runtime fingerprint diagnostics reproduce identically on immutable base and head and are unrelated. No merge performed.
…ge-1001 Signed-off-by: huangruiteng <huangrt01@163.com>
|
Updated the contributor branch with a signed merge of current main Fresh validation on this immutable head: 48 Python Host/Codex CLI cases and 10 native Host process cases passed with zero skips; changed-path Ruff, compile and diff hygiene checks passed. Goal-aware premerge passed all four applicable direct checks and verified the exact-diff quality receipt. The previous source-fingerprint baseline defect is now repaired in main by #5367. The original negative mutation evidence still applies to the unchanged atomic-publication fixture: interrupted publication preserves the previous complete marker, and real Host descendants retain SIGTERM resistance so timeout cleanup must actually stop them. No deadline or cancellation expectation was weakened. Public/private boundary scan is clean, no manual hold is identified, and no speculative companion refactor is needed. Requested a fresh review on the updated head; the earlier approval is not being reused as approval of the merge commit. CI waiting is disabled by the resolved review policy; local validation and exact-head merge readiness still apply. |
huangruiteng
left a comment
There was a problem hiding this comment.
评审精确 head:52a1ac56bd842434133e1c1ce5be2e5ae1d67a0b,更新后的主干基线:6a8a042ab868a0694e8c19a1ba29c534c575864a。这是同步主干后的独立维护者复核,按当前 capability policy revision 12 重新执行;没有沿用旧 head 的批准。
动机
旧 counter fixture 直接覆盖已发布文件,打开文件时先截断;清理若恰好杀进程于写入窗口,文件就留下空内容。后续存活检查把完整值变为空误当成仍在工作,给真实进程清理报假失败。当前任务是消除这个已复现的测试观测缺陷,同时保留对 Host 后代进程确实停止的验证;不是降低产品终止要求。
改动思路
先在同目录写完 staged counter,再以原子替换发布。中断发生在发布前,读者仍看到上一份完整值。只要求进程中断时的原子可见性,无需引入 fsync、产品重试或新生命周期 owner。通用 Host 与 Codex CLI 共用同一 source,生产 Python transport → TS Host supervisor → process-group cleanup 的决策、deadline 和结果合同均未改变。
具体改动
三个文件 +94/-28,全部是测试或 fixture。COUNTER_PROCESS_SOURCE 集中维护 PID 写入、SIGTERM 忽略、计数递增及 staged publication;可选 pause fence 只服务中断负例。test_counter_process_fixture_publishes_atomically 在明确 fence 后 kill/wait,检查已发布值完整,并在 finally 清理自有进程。两条通用 Host 消失/超时测试改用共享 source,保留原清理断言;_fake_codex 以 repr 注入 source、用 argv 传路径,由真实 child 写 PID,result 和 timeout 分支仍走实际 adapter/supervisor。
我重新执行负例:将 source 改为直接打开 marker,并在截断后暂停,原回归确定性失败为“空内容不等于已发布值”。独立正常运行控制也证明 counter 真正增长、SIGTERM 后仍增长、记录的 PID 等于真实进程、SIGKILL 后停止。这避免了“fixture 不工作,清理测试反而假通过”的反例。
对主干的风险
固定新 head 上,48 项 Python Host/Codex CLI、10 项真实 Node Host 测试通过,后者包括主干新加入的 closed-pipes 异步 KILL 完成边界。Ruff、编译、diff hygiene 通过;带 Goal 的 premerge 通过四个适用直接检查,严格质量回执 valid。Planner 对 tests-only 范围没有选 catalog smoke,不能把未选择写成已运行。此前的 source-fingerprint 基线故障已由 #5367 进入当前基线;相应三个历史失败用例在当前 head 也重跑通过,未被这条 PR 接管。
残余限制是本次验证为 macOS/POSIX 真实进程,未声明 Windows、所有 Python 版本或全量 pytest 覆盖;新增中断测试明确限定 POSIX。临时 staged 文件仅由测试临时目录持有,不新增产品状态。共享 source 已完成相邻边界的去重,无需再引入框架;现有覆盖与作者最近 15 条 PR 检查未发现同形 atomic-marker 重复。没有生产默认、权限、状态词表、机器义务、receipt 或前端交互改变,相关审查镜头不触发新的共享合同。
我的整体评价
结论 APPROVE。修复完整且可逆:关闭观测竞争,保留真实后代清理的反证能力。long_horizon 产品执行语义保持不变,普通 CLI、UI、Lark 用户旅程保持不变;维护者的验证体验因减少假失败而改善。不需要 UI companion 或数据迁移。这里批准的是当前整个补丁及主干整合结果,合并仍需当前 exact-head readiness 与用户授权;不以旧批准、测试数量或 CI 状态替代判断。
English verdict: APPROVE - HEAD 52a1ac5. Fresh review of the main-integrated tests-only atomic-marker fix: 48 Python and 10 real native Host cases passed, plus deterministic direct-write mutation rejection, signal/liveness controls, lint/compile/diff checks and strict exact-scope premerge. Production cancellation semantics and user entrypoints are unchanged. POSIX evidence only; no merge performed by this review.
Summary
os.replaceRoot cause
The Python fixtures used
Path.write_text()on the published marker. Opening the marker withmode="w"truncates it before writing, so process cleanup could land in that window and leave an empty marker. The liveness assertion then compared a complete value with the empty partial state and falsely reported that the descendant survived cleanup.Validation
assert "" == "published"under direct publicationtests/control_plane/test_host_process.py: 7 passedtests/test_loopx_turn_codex_cli.py: 41 passedgit diff --check: passedNo production code or dependency lockfile changed.