fix(validation): stop owned descendants on timeout and cancellation - #5268
Conversation
Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
huangruiteng
left a comment
There was a problem hiding this comment.
Approval conclusion (author-owned PR; GitHub blocks formal self-approval)
结论:APPROVE,没有阻断性发现。整份 PR 按 exact head 0cc62a0e7dfff8f0209eae71346f6a58bf541708 评审,基线为 526f250ad032303a4dab0b23a2d598a77817e926。这是“超时/取消不遗留验证子进程”的完整有界修复;不是长批次速度验收,也不改变原声明或期限。
动机
旧调用使用 subprocess.run,超时终止的只是 leader。子进程即使忽略 TERM 或在父进程退出后继续持有输出管道,也能在验证失败后继续运行、产生迟到效果。独立的信号应答检查在原基线复现了五个残留场景,其中两个经过真实 File/SQLite CLI。这个修复改善失败后继续工作的可靠性和用户看到的失败状态,但不能据此宣布整个长批次问题已解决。
改动思路
复用已有进程传输层,而不是再造一个完成决策源。调用方先经既有声明、工作树、源和租约见证检查,再在同一 Python interpreter 上执行批准的 argv;TS 仍负责完成准入和持久提交。正常退出返回原隐私回执,超时或取消则终止本命令拥有的进程组,原异常返回原调用方。零宽限直接 KILL,避免额外等待以及 TERM/KILL 竞态遮盖原超时。
我比较了不修改、只杀 leader、另建清理器、以及复用现有 helper 四种边界。只杀 leader 已被真实残留反例否定;另建清理器增加重复 OS 生命周期实现。将既有 helper 的零宽限分支修正并供验证器使用,是足以覆盖这次问题的较小实现。没有新增状态、设置、CLI 或同步维护字段。
具体改动
生产代码涉及两个模块,49 行增加、26 行删除;回归涉及两个测试文件,253 行增加、1 行删除;既有参考文档增加 33 行中英文说明。完整 diff 中没有生成资产、私有声明或新协议。进程运行参数由不透明字典改为同义的显式类型参数,stdin 线程捕获已窄化的 stream,保留原 provider 传输语义。
关键代码讲解
run_caller_validation,第 17 行:保留互斥 argv/文本、shell-free 解析、cwd、继承环境与 stdin。第 48–65 行使用 owned POSIX session;只有执行异常才调用共享清理器,然后重新抛出。正常/nonzero 退出仍返回完整原七字段回执,不输出命令日志或本地路径。_terminate_posix_process_group,第 33 行:零宽限只发送一次 group KILL;正宽限仍保留原 TERM→等待→KILL 路径。leader 已退出但 group 仍在时,也不会因为 leader 的 poll 结果跳过 group 清理。terminate_process_tree,第 78 行:公开既有 transport helper,限定调用方已拥有并隔离的进程;既有 capped provider 的 timeout/output-limit/cancellation 调用统一引用它,Windows 保留原 taskkill 适配器。该名称不授予 agent、managed worker 或持久协调权限。- 真实 CLI 回归,第 168 行:通过真实子进程和 File/SQLite 存储执行
todo complete,检查 typed timeout、失败回执、canonical Todo/revision 与私有声明不变,再独立检查子进程不能继续应答。另有 argv/文本×成功/失败、父进程先退出、取消和继承 stdin 回归;两个 mock 用例只检查信号分支,不能替代这些真实执行证据。
对主干的风险
共享 helper 还服务 extension execution/presentation,因此不能只测新增验证器。137 项本地回归覆盖完成声明首绑、源变化、digest、工作树、lane scope、terminal replay、provider 输出限额和隔离;全部通过、无跳过。新增测试的相同文件在未改生产代码的基线是 5 失败/5 通过,所有失败都是子进程仍在运行;本分支新增及 transport 回归 19 项通过。隔离安装 wheel 后又通过 10 项,包括 File/SQLite CLI,且运行模块哈希与 source 一致。上述集合有重叠,不当作独立证据数量累加。
最强剩余限制是主动脱离 owned group 的进程与 Windows containment;本次 macOS 测试不证明这两者。非交互验证命令不等于终端 job-control 验收。取消注入仅代替触发时机,实际 Popen、进程清理和存活检查均未 mock。长批次满足同步期限尚无本 PR 的证据,不能用这些测试或更改预算代替。回滚只需 revert 本切片,没有存储或历史回执迁移。
语义与 CI 对齐
TS 重构 RFC 的 terminal 边界允许 Python 执行显式 host validation effect,TS 保留声明、期限、源/租约围栏、完成决定和提交。本 PR 复用现有语义,不扩展词汇、权威或预算。评审 packet 的 wait_for_ci=false;未查询、轮询或等待远端 CI。采用仓库原生验证:四个改动 Python 文件 Ruff、两个生产模块 strict mypy、配置的 19 文件 mypy frontier、diff check,以及标准 Chat TypeScript/bundle 构建均通过。首次 wheel 构建因缺少生成 Chat 资产拒绝;完成标准构建后 wheel 成功,没有绕过打包门禁。
现有 Dashboard 继续读 canonical validator revision/digest 与 proposal failure,Lark 继续读同一失败投影;没有新字段、编辑器或 UI 状态权威。已检查相关源码并构建实际打包前端,不宣称新浏览器/Lark 传输体验经过验收。
我的整体评价
这是 justified increment:有独立可复现的用户问题、现有 owner 内的较小修复、真实 base/head 反例、安装产物验证与代码级回滚。长程工作维度得到改善——验证失败后不再留下可继续运行的 owned child;用户体验维度得到改善——原超时/失败保持真实,不被清理竞态误报为未启动。声明校验、失败保持开放、以后重试和结算权威均未放宽。
前向整理已经应用在同一 transport 边界:复用一个 helper、显式进程参数、稳定 stdin 引用;不增加平行 executor,也不为语言偏好扩大 TS 迁移。仍需在既有任务中分别验收长批次延迟、其有效声明与 maintainer 合并后的实际采用。Core 合并仍走维护者,本评审不是合并或本机安装授权。
English verdict: APPROVE - head 0cc62a0; owned POSIX validation children are stopped on timeout/cancellation with unchanged TS authority and receipts. Baseline: five reproduced leaks; head: 137 local regression cases passed, plus ten isolated-wheel cases. Long-batch deadline and Windows containment remain outside this qualification.
|
Frame-aligned conclusion at 按该 frame,本切片完成 owned-process 取消修复及真实 File/SQLite 读回;长批次同步期限、主动逃离进程组和新 Windows containment 验收仍未满足,文档和评审没有把这些行标成通过。Core 合并仍需维护者,live runtime 未替换。详细中文五段评审、19 项证据一致性检查与边界:exact-head review。 English verdict: APPROVE - head 0cc62a0; useful bounded owned-process cleanup, unchanged TS/receipt authority, 137 native cases and 10 isolated-wheel cases passed. This is not long-batch qualification or live adoption. |
Summary / 摘要
Roadmap: S10 reliability and S12 qualified delivery. Placement follows the TS migration RFC: this is a bounded execution/cancellation seam, not a second completion implementation or a new executor.
Changes / 改动
runtime/validation_command.py: preserve shell-free argv/text, cwd, inherited environment/stdin and privacy-safe receipts while using an owned POSIX group and shared cancellation cleanup.extensions/process_runtime.py: expose the existing cleanup helper, make zero-grace force-kill single-phase, and replace untyped process-option forwarding with equivalent typed options and a stable stdin stream reference.Validation / 验证
Unmodified baseline:
526f250ad032303a4dab0b23a2d598a77817e926. The identical 10-case regression file has 5 failures / 5 passes: all five failures show the owned child still running, including both real CLI/storage variants. No baseline assertions or production files were changed.git diff --check: passed.npm ci && npm run build:chat: TypeScript check and packaged Chat bundle build/verification passed. The first wheel attempt correctly rejected missing generated Chat assets; the normal build prepared them and the wheel then built successfully.Product entry points and boundaries / 产品入口与边界
CLI and managed caller validation change only process cancellation. Receipt schemas, validation controls and failure projection are unchanged. The existing Dashboard reads canonical validator revision/digest and proposal failure; existing Lark presentation consumes the same failure facts. No new field, selector, layout or UI source of truth is introduced. Frontend source was inspected and the packaged bundle was rebuilt; no new browser/Lark transport journey is claimed.
本次仅改变 CLI/managed 验证命令的取消行为;前端与 Lark 继续消费已有状态/失败投影,不新增配置或 UI 权威。已检查现有入口并按标准流程构建打包前端;不宣称新增浏览器或 Lark 传输验收。
This does not prove a long real-CLI batch fits its synchronous completion deadline. It does not revise a validator, widen a deadline, accept a timeout as progress, debit quota, promote a Goal or alter a storage provider. POSIX children that deliberately escape their group are outside this transport's containment boundary. Windows keeps the existing taskkill adapter and is not newly qualified by this macOS run. Interactive terminal job-control behavior is not qualified; validation commands are non-interactive.
Rollback is the code-only revert of this slice; no canonical schema, historical receipt or private declaration migration is needed. Core runtime changes remain maintainer-merge-required; no candidate adoption is performed here. Private state, baseline worktrees, generated assets, wheel/probe artifacts and raw execution logs stay excluded.