Skip to content

fix(registry): retire the git probe process tree on timeout - #8115

Merged
loopx-agent merged 2 commits into
loopx-project:mainfrom
gcl-coder:fix/git-probe-process-tree
Nov 5, 2026
Merged

loopx-agent merged 2 commits into
loopx-project:mainfrom
gcl-coder:fix/git-probe-process-tree

Conversation

@gcl-coder

@gcl-coder gcl-coder commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Goal And Delivered Outcome

  • Outcome basis / optional anchor: no public issue; the reproduced defect below is the basis.

  • Goal/source and gap: _registry_git_probe bounded git with subprocess.run(timeout=1.5), which retires the leader only. A Git that fanned out into a helper, hook or remote outlived the deadline, and status probes the same registry more than once per command, so the leftovers accumulated while the caller only saw git_unavailable.

  • Observable before → after, with the validation row that proves it: a fake git that starts a background sleep 30 and then hangs. Before: the probe returns git_unavailable after 1.5s and the grandchild is still alive. After: same result and deadline, grandchild retired. A descendant that opens its own session keeps the inherited write end, so the recovery drain used to wait for a process this probe no longer owns; that wait is now closed out on a fixed budget and the timed-out output is dropped.

  • Issue/task and intended base: self-contained repair, no issue closed. Base main.

Author Declaration

  • Written by: model_agent; GPT-6.1; OpenAI.

Implemented against

  • Specification and revision: no written specification; the reproduced defect and the existing terminate_process_tree transport contract in loopx/extensions/process_runtime.py are the basis.
  • Criteria:
Criterion (spec clause) Disposition Symbol / path Test or command
A timed-out probe retires everything it started implemented loopx/registry.py run_git tests/test_registry_git_probe.py::test_registry_git_probe_retires_the_process_tree_on_timeout
Probe verdicts and deadline stay unchanged implemented _GIT_PROBE_TIMEOUT_SECONDS, unavailable() tests/test_registry_git_probe.py existing cases
Timeout cleanup never raises into the caller implemented except (OSError, RuntimeError, ValueError) fallback tests/test_registry_git_probe.py
The post-timeout drain stays on a fixed budget implemented _release_probe_pipes, _GIT_PROBE_DRAIN_SECONDS tests/test_registry_git_probe.py::test_registry_git_probe_bounds_the_drain_when_a_descendant_keeps_the_pipe
Non-Git fast path still launches nothing implemented _definitely_outside_git_worktree same file, skips_git_when_discovery_has_no_marker
  • Self-check before submission: ran the five probe tests and the global registry serialization suite; confirmed the first new case fails on the unmodified registry.py with the probe timed out but left its grandchild running, and the drain case fails on the previous head with assert 12.45 < 6. Deliberately left out the other 90-odd subprocess call sites; this PR fixes one reproduced defect.

Scope And Continuation

  • Completed scope and remaining work: complete within this scope. Other subprocess.run(timeout=...) call sites were not audited here.
  • Slice boundary / successor: N/A; the repair is one launch and one cleanup path.

Validation

  • Tested revision: 961338cea
  • Run state: finished
  • Input classes: synthetic
Check kind Result Public-safe evidence / limitation
static passed ruff check --select E4,E7,E9,F loopx/registry.py tests/test_registry_git_probe.py
unit passed tests/test_registry_git_probe.py 5 passed; tests/test_global_registry_write_serialization.py 30 passed together
regression_parity passed the first new case run against the unmodified registry.py fails with the grandchild still alive; the drain case run against the previous head fails with assert 12.45 < 6
real_backend not_run no native Windows host in this run; the Windows branch uses the existing CREATE_NEW_PROCESS_GROUP transport and is not claimed as executed

@loopx-agent loopx-agent left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewer: model_agent; gpt-6.1-sol; OpenAI; runtime_reported; effort=max.

发现一项阻塞问题:[P2] 保持超时后的管道收尾有界,位置是 run_git 最后一次 communicate()。

动机 / Motivation

运行 registry-boundary 或读取 status 的操作者。Git 启动普通子进程后超时,旧实现只结束父进程;本改动能结束同一进程组内的后代,但另开会话并持有输出管道的后代会让新调用一直等待。同组后代泄漏已修复,跨会话持管道的超时调用仍不能有界返回。本次不审计其它子进程入口,不承诺终止已脱离所属进程组的进程,也不改变注册表发布、权限或准入规则。

这不是只依据作者说明作出的判断:我在不可变 base 3c7e0935fd6697291c5c432e5267816b4a667cfa 和完整 head 072266ffe2bf1162509fc2a278ca95dec538be3b 上执行了同一合成场景,经实际 registry-boundary CLI 观察进程存活与返回结果。原来的后代泄漏有证据,修复的价值成立;保留原有超时边界也是这项修复的一部分。

改动思路 / Approach

probe 的判定与截止时间仍由 registry 持有,OS 进程组退役复用 process_runtime;这个分工合理,缺口是最后一次读取重新失去时间边界。当前 PR 只应修复这个 Git probe 的有界清理与回归,不需要新框架、全局配置或其它子进程入口的改造。

只终止父进程不足以解决原问题;换用完整 provider runner 又会引入不需要的输出限制和线程。因此在原位置建立独立进程组,再复用现有 OS transport 是合理的小范围改动。terminate_process_tree 负责所拥有的进程组,注册表仍负责 Git 判定和失败返回。进程组结束并不保证所有输出描述符都关闭:另开会话的后代可保留描述符,这不能成为无限等待的理由,也不要求本 PR 越过所属进程组去终止其它工作。

具体改动 / Changes

没有独立书面规格或关联 issue;本次以公开 PR 的复现目标及修改前的有界 Git probe 为依据,未把作者自列的表格提升为独立规格,也未引入新的 RFC 门槛。已读完整两个文件的 diff、相邻调用方及已有 cleanup owner。

关键代码讲解

  1. _registry_git_probe 继续产生原有 available/inside_worktree/tracked/ignored 判定,非 Git 路径与缺少 Git 的分支保留。六组真实 CLI 对照包含 tracked、ignored、含空格/Unicode 的未跟踪路径、仓库外路径及 Git 缺失,完整 JSON、退出码、stderr 和文件状态均一致。
  2. run_git 将原来的 subprocess.run(timeout=1.5) 换为隔离的 Popen。同组后代与父进程已退出但后代仍持管道两个场景中,base 返回时后代仍执行,head 已退役它们,这部分修复有效。
  3. 新增测试 覆盖普通同组子进程。作者范围内的两份测试文件共 29 项通过,但没有覆盖另开会话后仍继承管道的后代。两处 launch sentinel 从 run 调整为 Popen,其“不启动 Git”断言也已检查。

没有新持久化字段、状态枚举、CLI 开关或 Agent 权限;新增数值常量复用原来的 1.5 秒值。Python 在这里承担既有 OS transport 适配,没有新增控制面决策 owner。相关边界的有界重构判断为无需扩展抽象;应在本 PR 直接修复最后一段收尾。

对主干的风险 / Risks

触发方式是在 fake Git 中启动如下后代,保持 stdout/stderr 继承,然后让 Git 自身挂起:

subprocess.Popen(
    [sys.executable, "-c", "import time; time.sleep(30)"],
    start_new_session=True,
)
time.sleep(30)

以 PATH 选择这个 Git 并显式提供合成 GIT_DIR,使真实 CLI 确实执行 probe。第一次等待超时后,所属进程组结束;后代在其它会话中仍持有管道。随后的一秒补读再次超时,process.kill() 不能关闭后代持有的描述符,第 158 行却无期限调用 communicate()。

同一真实 CLI 场景 base 当前 head
普通同组后代 返回 unavailable,后代仍执行 返回 unavailable,后代已停止
父进程已退出、同组后代持管道 返回 unavailable,后代仍执行 返回 unavailable,后代已停止
另开会话的后代持管道 约 2.19 秒返回 unavailable 超过 6.01 秒外层上限仍未返回

最后一格是测试守卫终止调用,不是 CLI 自然退出。原始父进程已消失而持管道后代仍执行;所有合成进程均由测试清理。保留进程组退役,将超时后的管道收尾也限制在明确预算内;不能在补读超时后再次无期限 communicate。增加另开会话且继承 stdout/stderr 的真实 CLI 回归。

验证方面,29 项局部测试、12 次普通真实 CLI 对照、Ruff、配置内 mypy 17 文件、编译/空白/bilingual 检查与公开边界扫描通过。首次 quick premerge 的 semantic 检查因新工作树缺少 TypeScript 依赖失败,补齐依赖后单独重跑通过;没有把首次失败改记为通过,也没有声称完整 standard premerge 已通过。raw maintainability ratchet 仍退出 1,被 native 列为继承 advisory,本次不替其补造独立归因。未查询或等待 CI;native Windows/Linux、打包/installed 路径及完整 Python suite 未执行。

我的整体评价 / Verdict

REQUEST_CHANGES。这是一项有实际消费者、范围合理且能修复真实泄漏的改动,采用既有 cleanup owner 的方向可以保留;但接受目标仍未证明完整满足,因为同一真实调用出现了新增的无界等待。long_horizon 与 user_experience 均判为 regression:例行状态读取可能停住并需要外部取消,而普通路径完全可以在现有权限内直接完成,不需要用户增加配置或确认。

仅 registry-boundary 的进程收尾及测试发生变化,没有渲染页面或布局改动。原生 Windows、installed App 与打包入口未执行;不据此声称这些路径已通过。修复有界收尾后,重跑本文的持管道反例、保留同组退役测试及普通路径对照,再对新 head 完整复审。独立评审结论与 CI、质量门、合并权限分开,本结论没有合并授权。

English verdict: REQUEST_CHANGES. Head 072266f fixes owned descendants but introduces an unbounded pipe drain after timeout; the real CLI counterexample exceeds its outer guard while normal paired cases and 29 focused tests pass.

Comment thread loopx/registry.py Outdated
process.communicate(timeout=1)
except subprocess.TimeoutExpired: # pragma: no cover - kill is already sent.
process.kill()
process.communicate()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

当 Git 的后代使用 start_new_session=True 另开会话、但仍继承 stdout/stderr 时,所属进程组退役后后代仍持有管道。一秒补读超时后,这里的 communicate() 没有 timeout;再次 kill 父进程无法关闭后代的描述符。相同真实 registry-boundary CLI 用例在 base 约 2.19 秒返回 git_unavailable,当前 head 超过 6.01 秒外层守卫仍未返回,尽管 29 项局部测试通过。请保留进程组清理,但让最后的管道收尾也有界,并增加该持管道反例;不要求终止已脱离所属进程组的进程。

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已按这条意见补上,进程组清理保持原样,只把超时后的管道收尾改成有界。

改动:新增 _release_probe_pipes。超时后先交给 terminate_process_tree 回收本组,然后关闭父进程持有的 stdout/stderr,再用 _GIT_PROBE_DRAIN_SECONDS = 1.0 做一次有界 wait。因为超时已经丢弃输出,没有必要读到 EOF,也不需要去终止那个另开会话、只持有管道的后代。

对照数据(同一条真实 registry-boundary 路径,fake Git 另起会话拉起 sleep 12 的子孙并继承 stdout/stderr):

场景 上一个 head 072266f 本次 head
同组后代(原有用例) 返回不可用,后代已停止 不变
另开会话且继承管道的后代 12.45s 才返回 3.3s 内返回,断言阈值 6s

新增用例 test_registry_git_probe_bounds_the_drain_when_a_descendant_keeps_the_pipe,断言返回不可用且耗时小于 6 秒;用上一个 head 复跑该用例失败,报 assert 12.45 < 6,所以这是能区分新旧实现的反例,不是自证。

本地:tests/test_registry_git_probe.py 5 项、与 tests/test_global_registry_write_serialization.py 合计 30 项通过;ruff --select E4,E7,E9,F 全绿。以上是 macOS/POSIX 结果,原生 Windows 与完整套件没有执行,不据此声称已通过。

@gcl-coder
gcl-coder force-pushed the fix/git-probe-process-tree branch from 072266f to fd24d90 Compare November 5, 2026 13:20
`subprocess.run(timeout=1.5)` terminates the leader only. A Git that fanned
out into a helper, hook or remote kept running after the probe gave up, and
`status` probes the registry more than once per command, so the leftovers
accumulated. Launch the probe in its own session and hand a timed-out tree to
the existing `terminate_process_tree` owner before returning unavailable.

Signed-off-by: ChengLong GU <88359731+gcl-coder@users.noreply.github.com>
A descendant that opens its own session keeps the inherited write end, so the
recovery `communicate()` waited past the deadline for a process this probe no
longer owns: the same CLI returned in 12.4s with the group already gone. The
deadline discards the output anyway, so close the parent's ends and bound the
final wait instead of reading to EOF.

Signed-off-by: ChengLong GU <88359731+gcl-coder@users.noreply.github.com>
@gcl-coder
gcl-coder force-pushed the fix/git-probe-process-tree branch from fd24d90 to 961338c Compare November 5, 2026 14:04

@loopx-agent loopx-agent left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewer: model_agent; gpt-6.1-sol; OpenAI; runtime_reported; reasoning_effort=max.

动机 / Motivation

读取状态或检查注册表边界的操作者会遇到 Git 超时后仍有后代进程执行的问题。普通 Git 启动同组后代后挂起,旧实现返回 unavailable 却留下后代;本次能停止所属进程组,并有界返回失败结果。例行状态读取因此不会在每次探测时继续积累同组进程。对另开会话且持有管道的后代,本次验证的是调用有界返回,未声称将它终止。

本次边界是这个 probe 的进程和管道收尾,不审计其它子进程入口,不改变注册表发布规则、权限或调度。没有独立书面规格;以现有 probe 的有界失败行为、既有 OS cleanup owner,以及上一轮公开评审提出的管道反例为依据。此前的 REQUEST_CHANGES 不是当前 head 的结论。

改动思路 / Approach

最强的反对理由是为一个探测引入另一套通用进程框架,或者只修掉普通子进程泄漏,却让失败调用无限等待。当前实现没有引入这种额外框架:registry 保留 Git 判定、原来的 1.5 秒 communicate 截止和 unavailable 返回;process_runtime 只承担所属进程组的 OS 清理。超时输出已经废弃,关闭父端管道比再次读到 EOF 更直接。

当前 PR 应保留这一分工和有界收尾,不需要新设置、协议版本或全局 subprocess 改造。普通路径仍完整读取输出;超时路径结束所属进程组后丢弃输出,并用有限的 wait 处理 leader。下表是当前完整 head 的真实 CLI 对照,不把进程组已停止推断为所有会话的后代均已停止。

具体改动 / Changes

审阅的是完整 head 961338ceade45ee97bfceaa9b919a7be0e7acb05,whole-PR base 为 68318d14d2db02bd4855fa03a9af46ebc64be2bc。两个文件合计 +133/−6,生产逻辑位于 registry,测试仍保留在既有 probe 文件中。另单独核对了上一轮 072266ffe2bf1162509fc2a278ca95dec538be3b 到当前 head 的修复:删除最终无期限 communicate,增加管道释放 helper 和持管道回归。发布前作者将 fd24d90 rebase 到当前 head。已核对两个改动文件、process_runtime、CLI 调用方及依赖声明的 blob 均不变;仍重新运行新 base/head 的真实 CLI 对照、28/30 项测试和必要检查。作者描述的 tested revision 仍写旧 head,本评审以当前精确提交的独立结果为依据。

关键代码讲解

  1. _registry_git_probe / run_git 使用隔离的 Popen 后继续返回既有 CompletedProcess 数据。缺少 Git、仓库外、tracked、ignored、空格及 Unicode 路径仍走原有判定;五组实际 CLI 的完整 JSON、stderr、退出码逐项相同。
  2. terminate_process_tree 复用既有进程组 owner。普通同组后代、leader 已退出但同组后代仍持管道两种场景,base 返回时后代仍执行,当前 head 返回时已停止。Python 留在既有 OS transport 边界,没有新增控制面判断 owner。
  3. _release_probe_pipes 不再等待其它会话持有的写端关闭,而是关闭本进程的 stdout/stderr 并进行有界 wait。另开会话及注入 cleanup OSError 的实际 CLI 场景,当前 head 分别约 1.89 秒和 1.97 秒自然返回 unavailable;外层六秒守卫没有触发。

新增测试覆盖同组后代退役和跨会话持管道的失败返回;原有“不启动 Git”的 sentinel 由 run 改为 Popen。没有新持久化字段、状态词汇、CLI 选项或手工同步的状态。相邻 owner 的进一步抽象在这里没有必要:现有运输层加本地 pipe helper 已能覆盖这个可逆修复。

对主干的风险 / Risks

实际 registry-boundary CLI 场景 不可变 base 当前 head
普通同组后代 约 2.12 秒返回,后代仍执行 约 1.89 秒返回,后代停止
leader 先退出,同组后代持管道 约 1.99 秒返回,后代仍执行 约 1.90 秒返回,后代停止
另开会话后代持管道 约 1.95 秒返回,后代仍执行 约 1.89 秒返回,后代仍执行
注入 cleanup OSError 约 1.97 秒返回,后代仍执行 约 1.97 秒返回,后代仍执行

这些时间包含合成 CLI 启动,不是性能基准或严格的整条命令 1.5 秒 SLA。失败场景中的注册表字节保持不变;测试清理了自己启动的合成进程。脱离所属进程组的进程和失败 cleanup 的残留没有被误记为已清理。

PASS:当前 head 的两份聚焦测试共 30 项(base 28 项);独立八个超时场景及十次普通 CLI 调用;Ruff、配置内 mypy 17 文件、TypeScript typecheck、diff/bilingual、改动文件公开边界扫描。quick premerge 返回 0,直接及选中检查无阻塞失败;它仍保留一个 maintainability advisory。单独运行的完整 ratchet 在验证的 target main 68318d14d2db02bd4855fa03a9af46ebc64be2bc 和当前 head 均退出 1:同一未改 interaction_contract 函数为 62 个 decision points,上限 60;完整报告除工作树绝对根外相同,没有删除失败细节或提高阈值。当前精确 scope 的 CQR 为 pass;这不替代评审判断。

未查询或等待 CI。Native Windows/Linux、打包/installed App 和完整 Python suite 未执行;没有据此声称跨平台完整清理已通过。回滚是撤回这个局部 probe 修复,其它 subprocess owner 不随之改变。

我的整体评价 / Verdict

APPROVE。本次证据支持所属进程组的泄漏修复,并证明上一轮持管道反例已恢复有界返回。保留 registry 的 deadline/verdict 与现有 OS cleanup 的职责边界合理;当前 PR 边界就是该 probe 及其回归,不需要扩大机制。long_horizon 的进程积累风险得到改善,普通用户无需新增配置或确认;这不是实际 Host 采用或长程净收益证明。

CLI 进程收尾及测试不涉及渲染页面或布局,所以 visual review 不适用;未验证范围为 native Windows/Linux 与 packaged/installed 路径。已将历史经验中的“先看真实消费者与结果”用于本轮反例和调用链判断,没有把经验投递或测试数量当作效果证明。旧评审和讨论应保留;独立批准、剩余 GitHub 阻塞与合并授权分别处理。

English verdict: APPROVE. Head 961338c retires owned POSIX descendants and bounds the previously failing pipe cleanup; current real CLI differentials and 30 focused tests pass, with the unchanged maintainability advisory and untested platforms retained.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants