Repository navigation
Conversation
Signed-off-by: aihop <4408344+hhyykk@users.noreply.github.com> (cherry picked from commit f5ace94af3690a002978628b02f56cdd59727adc) Signed-off-by: aihop <4408344+hhyykk@users.noreply.github.com>
loopx-agent
left a comment
There was a problem hiding this comment.
Reviewer: model_agent — gpt-6.1-sol / OpenAI; runtime_reported; reasoning_effort=max
REQUEST_CHANGES — 一个 P2:迁移被删除观察钩子的既有回收测试。 运行时修复方向有实证;当前相邻完整测试集仍出现本次删除直接引入的失败。
动机 / Motivation
维护者运行 smoke 或测试,随后清理其自有临时目录时会遇到这个问题。 例如主线程已退出、工作线程仍等候写文件时,旧实现先报告已停止;新实现应保留目录,等整个进程真正退出后再允许清理。 误报已停止会让清理与退出期写入交错,留下被重建的临时目录或丢失失败现场。
独立 Linux 实进程对照确认旧实现提前成功、新实现保持 pending,释放工作线程并回收后才返回 stopped。 运行时安全增量已得到实证,但一个依赖被删除观察钩子的既有测试在当前 head 失败,仍须修正其夹具并重验。 这次不交付新的 controller 会话、正式安装、持续运行稳定性或长程成本收益。
改动思路 / Approach
这个边界应复用原 _stop_effect_runtime,把“服务已交接”和“可以删除目录”分开。runtime.shutdown 的认证请求只是停机意图;wait_for_process_exit=True 必须继续等 PID 消失或对子进程完成真实回收。当前改动删除进程组快照捷径,没有新增状态、配置、provider 或审批轮次。保留这段删除比再维护一套按 ps 文本分类的推断更合适;普通 restart 的非严格分支原样保留。
原有停止 owner 通过删除错误捷径实现私有目录安全,普通交接另有现成契约;不增加状态或等待审批。 私有 namespace 删除前的真实停止不变量;修复相邻旧夹具后再批准,不扩 controller。Linux 上真实工作线程在停机返回后仍能写 witness 的对照说明它有独立价值,无须把完整双会话/controller 资格变成前置。下表是实际同输入观察;TCP peer 是合成的认证响应端,PID、线程、写文件和 waitpid 都是真实 OS 行为。
| 条件 | 不可变 base | 当前 head |
|---|---|---|
| 主线程 Z、工作线程仍活跃,独立进程组 | stopped,随后仍实际写文件 | shutdown_pending,退出并回收后 stopped |
| 相同情况、继承进程组 | stopped,随后仍实际写文件 | shutdown_pending,退出并回收后 stopped |
具体改动 / Changes
全文 base 23bb8c89d6a01794faf75330170cfc19402bb6ab 到 head 4a8b5058d229b4e766ef761813a5f9259d2f3207 共三条路径:production +1/-16,测试 +47/-2,双语开发文档 +12。没有机械搬移、生成文件或第二个决策 owner。
规范依据为 docs/development/testing-and-quality.md,spec_revision 23bb8c89d6a01794faf75330170cfc19402bb6ab。这里读的是本次修改之前已存在的 cleanup 条款;以下 NS 标签仅用于映射该段无编号要求:NS-EXIT implemented,真实退出前不允许清理;NS-RETAIN implemented,runner 失败保留目录和原输出且不跟随链接;NS-OPERATOR implemented,普通 restart 仍按服务交接返回。新增文档解释与此原要求一致,不能拿新增段落自证。
关键代码讲解
restart_effect_runtime默认wait_for_exit=False,显式严格调用才进入物理退出判断,未把全部普通重启升级为破坏性清理门。_stop_effect_runtime先发原认证 shutdown,再在严格分支执行 reap/PID 检查。删除ps捷径后,仍存在的 PID 保持 pending,locator 换人也不能让此分支提前成功。_reap_exited_runtime_child沿用非阻塞 waitpid。非本进程子节点不能冒领其回收责任;父进程真正回收后可重试,不靠“Z 就不会写”的文本假设。_retire_fixture_runtimes只读取自有新建 namespace 的合法 locator,严格结果未 stopped 即抛错,_remove_fixture_root因而不会继续删除。实际集成测试还覆盖 locator 替换/消失与有无独立组的组合。
对主干的风险 / Risks
[P2] 保留退出/回收语义,改掉依赖已删除钩子的夹具
当前 tests/control_plane/test_effect_runtime_restart.py::test_namespace_shutdown_reaps_child_exiting_during_group_observation 让 child 一直等 stdin,并且只在 monkeypatch 的 _posix_owned_group_has_exited 回调里关闭 stdin。删除该 helper 调用后,回调永不触发、child 从未被要求退出,最终得到 pending,stopped 断言失败。这是测试编排与本次删除不兼容,不能据此推断运行时仍会误删活跃 writer,也不能忽略它对当前 pytest 的破坏。
独立执行四个相邻完整模块得到 50 PASS / 1 FAIL;将唯一变化的 runtime owner 换成不可变 base,同一个未修改测试 1 PASS。该对照是 immutable owner 模块与相同测试/相邻依赖,未冒称完整 base 安装。最小修复是在本 PR 中让真实 child 的退出由独立同步事件触发,保留“退出后 reap、PID 消失、重复 waitpid 得 ChildProcessError”的实义断言,并重跑 uv run --extra test python -m pytest -q tests/control_plane/test_effect_runtime_restart.py tests/control_plane/test_runtime_retirement_cleanup.py tests/test_managed_effect_runtime_fixture.py tests/control_plane/test_effect_runtime_compile_cache.py;不要恢复已经证伪的 ps 捷径,也不要删掉有效回收覆盖。
语义与 CI 对齐
共享状态仍复用现有 stopped / shutdown_pending,没有新 vocabulary 或 prose 分类器。严格清理的失败是机器执行结果,文档没有把它说成可忽略建议;普通服务重启与严格目录删除的默认差异已经明确。按当前审查配置只做本地验证,未查询、等待或推断远端 CI。
当前 head 的完整 integration 模块 99 PASS / 1 Linux 平台 SKIP;独立 Linux arm64/Python 3.11 实进程对照补足上述线程场景,但没有称原 pytest Linux case 已执行:独立镜像没有 pytest。macOS/Python 3.14.8 的相邻完整集保留上述失败;Ruff 通过,standard premerge 6 direct + 11 selected PASS、0 hold。首次私有 Linux 探针对“继承组 base 应 pending”的预期错误已修正并保留;它是核验器问题,不是作者缺陷。测试通过与物理退出、真实 Node 缓存及长期稳定性仍分开。
我的整体评价 / Verdict
这是有正向价值、归属正确、体量适当的内部安全增量;long_horizon 与 user_experience 在所测停止/重试路径保持,未认证长期收益。没有新增 opt-in 能力,wait_for_exit=False 的默认路径已对照保持;公开协议没有扩大 actor 或 authority。当前 PR 边界保留为命名空间清理修复,不扩成 controller 重构。
仍需 REQUEST_CHANGES:修正现有测试对旧实现钩子的依赖并让完整相邻回收覆盖通过后再审。视觉检查不适用:所查为 runtime/runner/fixture,没有渲染改动;正式 App、Windows、真实 Node 退出缓存和长期效用仍未验证。没有合并、安装或 dismiss 权限推断。
English verdict: REQUEST_CHANGES at 4a8b5058d229b4e766ef761813a5f9259d2f3207. The process-exit safety fix is reproduced, but removing the group observer breaks an existing reaping test; preserve its semantic coverage and repair its trigger before approval.
| # before returning successful namespace cleanup. | ||
| _reap_exited_runtime_child(info) | ||
| break | ||
| # A zombie leader can still have threads writing into the namespace. |
There was a problem hiding this comment.
[P2] 删除该调用后,现有真实回收测试的 child 永远等 stdin;唯一关闭 stdin 的 monkeypatch 回调再也不执行。当前四模块50过1败,替换不可变 base owner 的同一测试过。请迁移退出触发、保留真实 waitpid/PID 断言;不要恢复已证伪的 ps 快捷成功或删除有效回收覆盖。
When a runtime's main thread becomes a zombie but another thread can still write, strict shutdown currently reports
stopped. A caller can then delete its namespace while work is still running.Remove the process-group snapshot shortcut from
wait_for_process_exit=True. Keep nonblocking child reaping and require the PID to disappear before reporting success. Ordinary restart retains its serving-locator behavior. Tests cover owned and inherited process groups, plus a real Linux zombie leader with a surviving writer.Validation on
4a8b5058d229b4e766ef761813a5f9259d2f3207(base1fe6134e0b0fd0df6dc671a9bce0ea29c9613891):uv run --extra test python -m pytest tests/control_plane/test_effect_runtime_integration.py -q100 passed, 1 warning in 92.06s (warning: Hypothesis cache collection).git diff --checkand RFC bilingual diff check passed.Limit: the zombie-thread regression is Linux-specific. A present or unreaped PID remains
shutdown_pending; callers must retain the namespace. This does not prove why an earlier historical full-suite timeout occurred.中文:修复“主线程已退出,但其他线程仍能写文件时误报停止”的问题。只有确认进程退出,才允许删除隔离目录。本 PR 独立面向 main,不包含 #7430 的统一契约实现;不授权合并或上线。
Target main was refreshed to
23bb8c89d6a01794faf75330170cfc19402bb6abbefore submission. Its only intervening commit changes status-vocabulary documentation and its test; the runtime paths in this PR are unchanged. Integration against future main remains a merge-time check.