From 611b5f5bd63ccb97f6e5a6b012e78866cd6793e9 Mon Sep 17 00:00:00 2001 From: ChengLong GU <88359731+gcl-coder@users.noreply.github.com> Date: Thu, 5 Nov 2026 18:53:21 +0800 Subject: [PATCH 1/2] fix(registry): retire the git probe process tree on timeout `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> --- loopx/registry.py | 33 ++++++++++++++++++++--- tests/test_registry_git_probe.py | 46 ++++++++++++++++++++++++++++++-- 2 files changed, 73 insertions(+), 6 deletions(-) diff --git a/loopx/registry.py b/loopx/registry.py index a8826530fd..c440eeaa81 100644 --- a/loopx/registry.py +++ b/loopx/registry.py @@ -95,6 +95,9 @@ def registry_goals(registry: dict[str, Any]) -> list[dict[str, Any]]: ) +_GIT_PROBE_TIMEOUT_SECONDS = 1.5 + + def _definitely_outside_git_worktree(probe_dir: Path) -> bool: if os.environ.get("GIT_DIR") or os.environ.get("GIT_WORK_TREE"): return False @@ -119,8 +122,11 @@ def unavailable() -> dict[str, Any]: } def run_git(*args: str, cwd: Path = probe_dir) -> subprocess.CompletedProcess[str] | None: + # A probe that times out must retire everything it started. `run` kills + # only the leader, so a Git that fanned out into a helper, a hook or a + # remote would outlive the deadline on every later status read. try: - return subprocess.run( + process = subprocess.Popen( ["git", *args], cwd=cwd, text=True, @@ -128,11 +134,30 @@ def run_git(*args: str, cwd: Path = probe_dir) -> subprocess.CompletedProcess[st errors="replace", stdout=subprocess.PIPE, stderr=subprocess.PIPE, - check=False, - timeout=1.5, + start_new_session=os.name == "posix", + creationflags=( + int(getattr(subprocess, "CREATE_NEW_PROCESS_GROUP")) + if os.name == "nt" else 0 + ), ) - except (OSError, subprocess.TimeoutExpired): + except OSError: + return None + try: + stdout, stderr = process.communicate(timeout=_GIT_PROBE_TIMEOUT_SECONDS) + except subprocess.TimeoutExpired: + from .extensions.process_runtime import terminate_process_tree + + try: + terminate_process_tree(process, 0) + except (OSError, RuntimeError, ValueError): + process.kill() + try: + process.communicate(timeout=1) + except subprocess.TimeoutExpired: # pragma: no cover - kill is already sent. + process.kill() + process.communicate() return None + return subprocess.CompletedProcess(["git", *args], process.returncode, stdout, stderr) # Git discovers a normal repository only through an ancestor `.git` # marker. Non-Git LoopX projects are common, and status can inspect the diff --git a/tests/test_registry_git_probe.py b/tests/test_registry_git_probe.py index f87e646711..b380494677 100644 --- a/tests/test_registry_git_probe.py +++ b/tests/test_registry_git_probe.py @@ -1,6 +1,9 @@ from __future__ import annotations +import os +import signal import subprocess +import time from pathlib import Path import pytest @@ -35,7 +38,7 @@ def unexpected_git( calls.append(tuple(command)) return subprocess.CompletedProcess(command, 128, "", "not a git repository") - monkeypatch.setattr(registry_module.subprocess, "run", unexpected_git) + monkeypatch.setattr(registry_module.subprocess, "Popen", unexpected_git) assert registry_module._registry_git_probe(registry_path) == { "available": True, @@ -102,7 +105,7 @@ def test_registry_git_probe_reports_unavailable_without_git( def unexpected_git(*_: object, **__: object) -> None: raise AssertionError("non-Git fast path must not launch Git") - monkeypatch.setattr(registry_module.subprocess, "run", unexpected_git) + monkeypatch.setattr(registry_module.subprocess, "Popen", unexpected_git) assert registry_module._registry_git_probe(registry_path) == { "available": False, @@ -112,3 +115,42 @@ def unexpected_git(*_: object, **__: object) -> None: "ignored": False, "worktree_root_recorded": False, } + + +@pytest.mark.skipif(os.name != "posix", reason="the probe's fake git is a POSIX shell script") +def test_registry_git_probe_retires_the_process_tree_on_timeout( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A Git that fans out must not outlive the deadline that ended the probe.""" + + probe = tmp_path / "project" + probe.mkdir() + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + grandchild = tmp_path / "grandchild.pid" + fake_git = bin_dir / "git" + fake_git.write_text( + "#!/bin/sh\n" + "sleep 30 &\n" + f"echo $! > {grandchild}\n" + "sleep 30\n", + encoding="utf-8", + ) + fake_git.chmod(0o755) + monkeypatch.setenv("PATH", f"{bin_dir}{os.pathsep}{os.environ['PATH']}") + monkeypatch.setenv("GIT_DIR", str(tmp_path / "fake.git")) + + observed = registry_module._registry_git_probe(probe / "registry.json") + assert observed["probe_status"] == "git_unavailable" + + pid = int(grandchild.read_text(encoding="utf-8").strip()) + deadline = time.monotonic() + 10 + while time.monotonic() < deadline: + try: + os.kill(pid, 0) + except ProcessLookupError: + return + time.sleep(0.05) + os.kill(pid, signal.SIGKILL) + raise AssertionError("the probe timed out but left its grandchild running") From 961338ceade45ee97bfceaa9b919a7be0e7acb05 Mon Sep 17 00:00:00 2001 From: ChengLong GU <88359731+gcl-coder@users.noreply.github.com> Date: Thu, 5 Nov 2026 21:03:47 +0800 Subject: [PATCH 2/2] fix(registry): drop a timed-out probe's pipes instead of draining them 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> --- loopx/registry.py | 28 +++++++++++++++++---- tests/test_registry_git_probe.py | 42 ++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 5 deletions(-) diff --git a/loopx/registry.py b/loopx/registry.py index c440eeaa81..d944d9f3e3 100644 --- a/loopx/registry.py +++ b/loopx/registry.py @@ -96,6 +96,7 @@ def registry_goals(registry: dict[str, Any]) -> list[dict[str, Any]]: _GIT_PROBE_TIMEOUT_SECONDS = 1.5 +_GIT_PROBE_DRAIN_SECONDS = 1.0 def _definitely_outside_git_worktree(probe_dir: Path) -> bool: @@ -108,6 +109,27 @@ def _definitely_outside_git_worktree(probe_dir: Path) -> bool: return not any(os.path.lexists(parent / ".git") for parent in (current, *current.parents)) +def _release_probe_pipes(process: subprocess.Popen[str]) -> None: + """Drop a timed-out probe's pipes instead of draining them. + + A descendant that opened its own session keeps the inherited write end, so + reading to EOF waits for a process this probe no longer owns. The deadline + already discards the output, so close the parent's ends and let the one + reaped leader decide how long the release may take. + """ + + for stream in (process.stdout, process.stderr): + try: + if stream is not None: + stream.close() + except (OSError, ValueError): # pragma: no cover - already closed. + pass + try: + process.wait(timeout=_GIT_PROBE_DRAIN_SECONDS) + except subprocess.TimeoutExpired: # pragma: no cover - the group kill already landed. + process.kill() + + def _registry_git_probe(path: Path) -> dict[str, Any]: probe_dir = path if path.is_dir() else path.parent @@ -151,11 +173,7 @@ def run_git(*args: str, cwd: Path = probe_dir) -> subprocess.CompletedProcess[st terminate_process_tree(process, 0) except (OSError, RuntimeError, ValueError): process.kill() - try: - process.communicate(timeout=1) - except subprocess.TimeoutExpired: # pragma: no cover - kill is already sent. - process.kill() - process.communicate() + _release_probe_pipes(process) return None return subprocess.CompletedProcess(["git", *args], process.returncode, stdout, stderr) diff --git a/tests/test_registry_git_probe.py b/tests/test_registry_git_probe.py index b380494677..24721dd692 100644 --- a/tests/test_registry_git_probe.py +++ b/tests/test_registry_git_probe.py @@ -3,6 +3,7 @@ import os import signal import subprocess +import sys import time from pathlib import Path @@ -154,3 +155,44 @@ def test_registry_git_probe_retires_the_process_tree_on_timeout( time.sleep(0.05) os.kill(pid, signal.SIGKILL) raise AssertionError("the probe timed out but left its grandchild running") + + +@pytest.mark.skipif(os.name != "posix", reason="the probe's fake git is a POSIX script") +def test_registry_git_probe_bounds_the_drain_when_a_descendant_keeps_the_pipe( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A descendant in another session can hold stdout open past the deadline.""" + + probe = tmp_path / "project" + probe.mkdir() + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + grandchild = tmp_path / "grandchild.pid" + fake_git = bin_dir / "git" + fake_git.write_text( + f"#!{sys.executable}\n" + "import subprocess, sys, time\n" + "child = subprocess.Popen([sys.executable, '-c', 'import time; time.sleep(12)'],\n" + " start_new_session=True)\n" + f"open({str(grandchild)!r}, 'w', encoding='utf-8').write(str(child.pid))\n" + "time.sleep(12)\n", + encoding="utf-8", + ) + fake_git.chmod(0o755) + monkeypatch.setenv("PATH", f"{bin_dir}{os.pathsep}{os.environ['PATH']}") + monkeypatch.setenv("GIT_DIR", str(tmp_path / "fake.git")) + + started = time.monotonic() + try: + observed = registry_module._registry_git_probe(probe / "registry.json") + elapsed = time.monotonic() - started + finally: + if grandchild.exists(): + try: + os.kill(int(grandchild.read_text(encoding="utf-8").strip() or 0), signal.SIGKILL) + except (OSError, ValueError): + pass + + assert observed["probe_status"] == "git_unavailable" + assert elapsed < 6, f"the probe waited {elapsed:.2f}s on a pipe it no longer owns"