From 953b4f8da72401cbde31ae8a506cf13c9711dfd8 Mon Sep 17 00:00:00 2001 From: ChengLong GU <88359731+gcl-coder@users.noreply.github.com> Date: Fri, 6 Nov 2026 07:38:16 +0800 Subject: [PATCH 1/3] fix(lark): hold the collector's Git reads to one budget Admitting a collector config path asks Git three questions -- whether the project is a worktree, whether the file is tracked, and whether it is ignored -- and none of them carried a budget. A Git blocked on a stale index lock, a network mount or an oversized index therefore held the command open with no answer at all; reproduced here with a fake `git` on PATH that only sleeps, where `_project_config_path` was still running after 5s. The budget alone was not enough: a `TimeoutExpired` would have escaped the admission check as a traceback, so the refusal paths had to say what "Git did not answer" means. Acceptance needs proof that the config is ignored and untracked, so an unanswered probe refuses the path. The three reads share `GIT_PROBE_TIMEOUT_SECONDS` through `_run_git_probe`, and a Git that exits non-zero still takes the existing boundary and non-Git fallbacks unchanged. The queries are local reads that never fan out into helpers, so the leader-only kill that `subprocess.run` performs covers them. Signed-off-by: ChengLong GU <88359731+gcl-coder@users.noreply.github.com> --- loopx/extensions/lark/event_collector.py | 50 +++++--- tests/test_lark_event_collector_git_budget.py | 110 ++++++++++++++++++ 2 files changed, 145 insertions(+), 15 deletions(-) create mode 100644 tests/test_lark_event_collector_git_budget.py diff --git a/loopx/extensions/lark/event_collector.py b/loopx/extensions/lark/event_collector.py index 87cf42a42e..26060c39e9 100644 --- a/loopx/extensions/lark/event_collector.py +++ b/loopx/extensions/lark/event_collector.py @@ -40,6 +40,34 @@ Runner = Callable[..., subprocess.CompletedProcess[str]] +# The collector accepts a config path only when Git proves it is ignored and +# untracked. That proof costs three Git reads, and a Git blocked on a stale +# index lock, a network mount or an oversized index would otherwise hold the +# command open with no answer at all. +GIT_PROBE_TIMEOUT_SECONDS = 5.0 + + +def _run_git_probe(root: Path, *arguments: str) -> subprocess.CompletedProcess[str] | None: + """Ask Git one ownership question, answering ``None`` when it cannot. + + No answer is not a "no" for this caller: acceptance needs proof that the + config is ignored and untracked, so a probe that runs out of budget lands on + the refusing side instead of raising out of the command. + """ + + try: + return subprocess.run( + ["git", "-C", str(root), *arguments], + text=True, + encoding="utf-8", + errors="replace", + capture_output=True, + check=False, + timeout=GIT_PROBE_TIMEOUT_SECONDS, + ) + except (OSError, subprocess.SubprocessError): + return None + def _project_config_path(project: str | Path, raw: str | Path) -> Path: root = Path(project).expanduser().resolve() @@ -50,11 +78,9 @@ def _project_config_path(project: str | Path, raw: str | Path) -> Path: relative = path.relative_to(root) except ValueError as exc: raise ValueError("lark collector config must stay inside the project") from exc - git_probe = subprocess.run( - ["git", "-C", str(root), "rev-parse", "--show-toplevel"], - capture_output=True, - check=False, - ) + git_probe = _run_git_probe(root, "rev-parse", "--show-toplevel") + if git_probe is None: + raise ValueError("lark collector config needs a Git answer the probe could not give") if git_probe.returncode != 0: has_git_boundary = any( (candidate / ".git").exists() for candidate in (root, *root.parents) @@ -65,16 +91,10 @@ def _project_config_path(project: str | Path, raw: str | Path) -> Path: "under .loopx/config in a non-Git project" ) return path - tracked = subprocess.run( - ["git", "-C", str(root), "ls-files", "--error-unmatch", "--", str(relative)], - capture_output=True, - check=False, - ) - ignored = subprocess.run( - ["git", "-C", str(root), "check-ignore", "--quiet", "--", str(relative)], - capture_output=True, - check=False, - ) + tracked = _run_git_probe(root, "ls-files", "--error-unmatch", "--", str(relative)) + ignored = _run_git_probe(root, "check-ignore", "--quiet", "--", str(relative)) + if tracked is None or ignored is None: + raise ValueError("lark collector config needs a Git answer the probe could not give") if tracked.returncode == 0 or ignored.returncode != 0: raise ValueError("lark collector config must be ignored and untracked") return path diff --git a/tests/test_lark_event_collector_git_budget.py b/tests/test_lark_event_collector_git_budget.py new file mode 100644 index 0000000000..98ebf5214b --- /dev/null +++ b/tests/test_lark_event_collector_git_budget.py @@ -0,0 +1,110 @@ +"""The Lark collector's config admission asks Git three questions. + +Each of those reads used to be unbounded, so a Git blocked on a stale index +lock, a network mount or an oversized index held the command open with no +answer. These tests pin the budget and the refusal that follows it. +""" + +from __future__ import annotations + +import os +import subprocess +import time +from pathlib import Path + +import pytest + +from loopx.extensions.lark import event_collector +from loopx.extensions.lark.event_collector import _project_config_path + + +def _git_project(tmp_path: Path) -> Path: + project = tmp_path / "project" + project.mkdir() + subprocess.run(["git", "init", "-q", str(project)], check=True) + (project / ".gitignore").write_text(".loopx/\n", encoding="utf-8") + return project + + +def _completed(root: Path, returncode: int) -> subprocess.CompletedProcess[str]: + return subprocess.CompletedProcess( + args=["git"], + returncode=returncode, + stdout=f"{root}\n" if returncode == 0 else "", + stderr="", + ) + + +def test_an_unanswered_rev_parse_refuses_instead_of_raising( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + project = _git_project(tmp_path) + target = project / ".loopx" / "config" / "collector.json" + target.parent.mkdir(parents=True) + target.write_text("{}", encoding="utf-8") + + def no_answer(*args: object, **kwargs: object) -> None: + raise subprocess.TimeoutExpired(cmd="git", timeout=5.0) + + monkeypatch.setattr(event_collector.subprocess, "run", no_answer) + + with pytest.raises(ValueError, match="needs a Git answer"): + _project_config_path(project, ".loopx/config/collector.json") + + +def test_an_unanswered_ignore_probe_refuses_the_config( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + project = _git_project(tmp_path) + target = project / ".loopx" / "config" / "collector.json" + target.parent.mkdir(parents=True) + target.write_text("{}", encoding="utf-8") + real_run = subprocess.run + + def bounded(command: list[str], **kwargs: object) -> subprocess.CompletedProcess[str]: + if command[-1] == "--show-toplevel": + return _completed(project, 0) + if command[3] == "ls-files": + return _completed(project, 1) + raise subprocess.TimeoutExpired(cmd="git", timeout=kwargs["timeout"]) + + monkeypatch.setattr(event_collector.subprocess, "run", bounded) + try: + with pytest.raises(ValueError, match="needs a Git answer"): + _project_config_path(project, ".loopx/config/collector.json") + finally: + monkeypatch.setattr(event_collector.subprocess, "run", real_run) + + +@pytest.mark.skipif(os.name != "posix", reason="the fake Git is a POSIX shell script") +def test_a_git_that_never_answers_stays_inside_the_budget( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + project = _git_project(tmp_path) + target = project / ".loopx" / "config" / "collector.json" + target.parent.mkdir(parents=True) + target.write_text("{}", encoding="utf-8") + fake_bin = tmp_path / "fakebin" + fake_bin.mkdir() + fake_git = fake_bin / "git" + fake_git.write_text("#!/bin/sh\nsleep 30\n", encoding="utf-8") + fake_git.chmod(0o755) + + monkeypatch.setenv("PATH", os.pathsep.join([str(fake_bin), os.environ["PATH"]])) + monkeypatch.setattr(event_collector, "GIT_PROBE_TIMEOUT_SECONDS", 0.3) + + started = time.monotonic() + with pytest.raises(ValueError, match="needs a Git answer"): + _project_config_path(project, ".loopx/config/collector.json") + elapsed = time.monotonic() - started + + assert elapsed < 5.0, f"the probe held the command open for {elapsed:.2f}s" + + +def test_an_ignored_untracked_config_still_loads(tmp_path: Path) -> None: + project = _git_project(tmp_path) + target = project / ".loopx" / "config" / "collector.json" + target.parent.mkdir(parents=True) + target.write_text("{}", encoding="utf-8") + + assert _project_config_path(project, ".loopx/config/collector.json") == target.resolve() From 23b532bb37438d308aeef5d337ff21e313ed1b74 Mon Sep 17 00:00:00 2001 From: ChengLong GU <88359731+gcl-coder@users.noreply.github.com> Date: Fri, 6 Nov 2026 10:21:43 +0800 Subject: [PATCH 2/3] fix(lark): stop the collector's Git questions together with its budget Two gaps in the config admission. The budget ended only the Git leader. Git starts helpers of its own -- an fsmonitor hook is the common one on `ls-files` -- inside the group it runs in, so a blocked admission returned its refusal while the hook and its child kept running, and repeated runs accumulated them. The probe now runs in a session of its own, and an exhausted budget retires that group through `terminate_process_tree` before dropping the pipes instead of draining them. The three questions each carried a fresh budget. Once the tracked query had timed out the admission could no longer be satisfied, yet `check-ignore` was still started and spent another full budget on the same hang. The questions now share one deadline, an unanswered tracked query refuses at once, and a question asked after the deadline has passed is not started at all. Signed-off-by: ChengLong GU <88359731+gcl-coder@users.noreply.github.com> --- loopx/extensions/lark/event_collector.py | 84 +++++++++-- tests/test_lark_event_collector_git_budget.py | 142 ++++++++++++------ 2 files changed, 168 insertions(+), 58 deletions(-) diff --git a/loopx/extensions/lark/event_collector.py b/loopx/extensions/lark/event_collector.py index 26060c39e9..3befdd967f 100644 --- a/loopx/extensions/lark/event_collector.py +++ b/loopx/extensions/lark/event_collector.py @@ -8,6 +8,7 @@ import shlex import shutil import subprocess +import time from collections.abc import Callable, Mapping, Sequence from pathlib import Path from typing import Any @@ -18,6 +19,7 @@ inspect_lark_event_inbox, load_lark_event_inbox_config, ) +from ..process_runtime import terminate_process_tree from .identity_shapes import LARK_CHAT_ID_PATTERN CONFIG_SCHEMA_VERSION_V0 = "lark_event_collector_config_v0" @@ -47,26 +49,75 @@ GIT_PROBE_TIMEOUT_SECONDS = 5.0 -def _run_git_probe(root: Path, *arguments: str) -> subprocess.CompletedProcess[str] | None: +def _retire_owned_probe(process: subprocess.Popen[str]) -> None: + """Stop this probe's own process group, then drop its pipes unread. + + Git starts helpers of its own -- an fsmonitor hook is the common one on + ``ls-files`` -- inside the group it runs in, so ending only the Git process + leaves them behind for the next run to inherit. An inherited pipe also + outlives the leader, so the pipes are closed instead of drained. + """ + + try: + terminate_process_tree(process, grace_seconds=0.2) + except (OSError, RuntimeError): + pass + finally: + for stream in (process.stdout, process.stderr): + if stream is not None: + stream.close() + try: + process.wait(timeout=1.0) + except subprocess.TimeoutExpired: + pass + + +def _git_deadline() -> float: + """The moment the whole admission check stops asking Git questions.""" + + return time.monotonic() + GIT_PROBE_TIMEOUT_SECONDS + + +def _run_git_probe( + root: Path, *arguments: str, deadline: float | None = None +) -> subprocess.CompletedProcess[str] | None: """Ask Git one ownership question, answering ``None`` when it cannot. No answer is not a "no" for this caller: acceptance needs proof that the config is ignored and untracked, so a probe that runs out of budget lands on - the refusing side instead of raising out of the command. + the refusing side instead of raising out of the command. The three + questions share one deadline, and a question asked after it has passed is + not started at all. """ + budget = ( + GIT_PROBE_TIMEOUT_SECONDS + if deadline is None + else deadline - time.monotonic() + ) + if budget <= 0: + return None + argv = ["git", "-C", str(root), *arguments] try: - return subprocess.run( - ["git", "-C", str(root), *arguments], + process = subprocess.Popen( + argv, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, text=True, encoding="utf-8", errors="replace", - capture_output=True, - check=False, - timeout=GIT_PROBE_TIMEOUT_SECONDS, + start_new_session=os.name == "posix", ) - except (OSError, subprocess.SubprocessError): + except OSError: + return None + try: + stdout, _stderr = process.communicate(timeout=budget) + except subprocess.TimeoutExpired: + _retire_owned_probe(process) return None + return subprocess.CompletedProcess( + args=argv, returncode=process.returncode, stdout=stdout, stderr="" + ) def _project_config_path(project: str | Path, raw: str | Path) -> Path: @@ -78,7 +129,8 @@ def _project_config_path(project: str | Path, raw: str | Path) -> Path: relative = path.relative_to(root) except ValueError as exc: raise ValueError("lark collector config must stay inside the project") from exc - git_probe = _run_git_probe(root, "rev-parse", "--show-toplevel") + deadline = _git_deadline() + git_probe = _run_git_probe(root, "rev-parse", "--show-toplevel", deadline=deadline) if git_probe is None: raise ValueError("lark collector config needs a Git answer the probe could not give") if git_probe.returncode != 0: @@ -91,9 +143,17 @@ def _project_config_path(project: str | Path, raw: str | Path) -> Path: "under .loopx/config in a non-Git project" ) return path - tracked = _run_git_probe(root, "ls-files", "--error-unmatch", "--", str(relative)) - ignored = _run_git_probe(root, "check-ignore", "--quiet", "--", str(relative)) - if tracked is None or ignored is None: + tracked = _run_git_probe( + root, "ls-files", "--error-unmatch", "--", str(relative), deadline=deadline + ) + if tracked is None: + # Acceptance already failed; asking the next question would only spend + # what is left of the same budget on the same hang. + raise ValueError("lark collector config needs a Git answer the probe could not give") + ignored = _run_git_probe( + root, "check-ignore", "--quiet", "--", str(relative), deadline=deadline + ) + if ignored is None: raise ValueError("lark collector config needs a Git answer the probe could not give") if tracked.returncode == 0 or ignored.returncode != 0: raise ValueError("lark collector config must be ignored and untracked") diff --git a/tests/test_lark_event_collector_git_budget.py b/tests/test_lark_event_collector_git_budget.py index 98ebf5214b..ed4682bfaa 100644 --- a/tests/test_lark_event_collector_git_budget.py +++ b/tests/test_lark_event_collector_git_budget.py @@ -2,12 +2,14 @@ Each of those reads used to be unbounded, so a Git blocked on a stale index lock, a network mount or an oversized index held the command open with no -answer. These tests pin the budget and the refusal that follows it. +answer. These tests pin the shared budget, the early refusal, and the process +tree a blocked read would otherwise leave behind. """ from __future__ import annotations import os +import shlex import subprocess import time from pathlib import Path @@ -26,71 +28,80 @@ def _git_project(tmp_path: Path) -> Path: return project -def _completed(root: Path, returncode: int) -> subprocess.CompletedProcess[str]: - return subprocess.CompletedProcess( - args=["git"], - returncode=returncode, - stdout=f"{root}\n" if returncode == 0 else "", - stderr="", - ) +def _config(project: Path) -> Path: + target = project / ".loopx" / "config" / "collector.json" + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text("{}", encoding="utf-8") + return target + + +def _fake_git(tmp_path: Path, script: str, monkeypatch: pytest.MonkeyPatch) -> Path: + bin_dir = tmp_path / "fakebin" + bin_dir.mkdir(exist_ok=True) + fake = bin_dir / "git" + fake.write_text(script, encoding="utf-8") + fake.chmod(0o755) + monkeypatch.setenv("PATH", f"{bin_dir}{os.pathsep}{os.environ['PATH']}") + return bin_dir -def test_an_unanswered_rev_parse_refuses_instead_of_raising( +def test_a_git_that_cannot_start_refuses_the_config( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: project = _git_project(tmp_path) - target = project / ".loopx" / "config" / "collector.json" - target.parent.mkdir(parents=True) - target.write_text("{}", encoding="utf-8") + _config(project) - def no_answer(*args: object, **kwargs: object) -> None: - raise subprocess.TimeoutExpired(cmd="git", timeout=5.0) + def cannot_start(command, **kwargs): + raise OSError("git is not installed") - monkeypatch.setattr(event_collector.subprocess, "run", no_answer) + monkeypatch.setattr(event_collector.subprocess, "Popen", cannot_start) with pytest.raises(ValueError, match="needs a Git answer"): _project_config_path(project, ".loopx/config/collector.json") -def test_an_unanswered_ignore_probe_refuses_the_config( +@pytest.mark.skipif(os.name != "posix", reason="the fake git is a POSIX script") +def test_the_next_question_is_not_asked_after_one_goes_unanswered( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: project = _git_project(tmp_path) - target = project / ".loopx" / "config" / "collector.json" - target.parent.mkdir(parents=True) - target.write_text("{}", encoding="utf-8") - real_run = subprocess.run + _config(project) + asked = tmp_path / "asked.txt" + asked.write_text("", encoding="utf-8") + _fake_git( + tmp_path, + "#!/bin/sh\n" + f'echo "$*" >> {shlex.quote(str(asked))}\n' + 'for argument in "$@"; do\n' + ' case "$argument" in *--error-unmatch*) sleep 30 ;; esac\n' + "done\n" + 'if [ "$3" = "rev-parse" ]; then printf "%s\\n" "$2"; exit 0; fi\n' + "exit 0\n", + monkeypatch, + ) + monkeypatch.setattr(event_collector, "GIT_PROBE_TIMEOUT_SECONDS", 0.5) - def bounded(command: list[str], **kwargs: object) -> subprocess.CompletedProcess[str]: - if command[-1] == "--show-toplevel": - return _completed(project, 0) - if command[3] == "ls-files": - return _completed(project, 1) - raise subprocess.TimeoutExpired(cmd="git", timeout=kwargs["timeout"]) + started = time.monotonic() + with pytest.raises(ValueError, match="needs a Git answer"): + _project_config_path(project, ".loopx/config/collector.json") + elapsed = time.monotonic() - started - monkeypatch.setattr(event_collector.subprocess, "run", bounded) - try: - with pytest.raises(ValueError, match="needs a Git answer"): - _project_config_path(project, ".loopx/config/collector.json") - finally: - monkeypatch.setattr(event_collector.subprocess, "run", real_run) + commands = asked.read_text(encoding="utf-8") + assert "rev-parse" in commands + assert "--error-unmatch" in commands + # The ignore flag of an unproven config cannot change the answer, so the + # third read is not started at all. + assert "check-ignore" not in commands + assert elapsed < 10, f"the admission spent {elapsed:.1f}s on two questions" -@pytest.mark.skipif(os.name != "posix", reason="the fake Git is a POSIX shell script") +@pytest.mark.skipif(os.name != "posix", reason="the fake git is a POSIX script") def test_a_git_that_never_answers_stays_inside_the_budget( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: project = _git_project(tmp_path) - target = project / ".loopx" / "config" / "collector.json" - target.parent.mkdir(parents=True) - target.write_text("{}", encoding="utf-8") - fake_bin = tmp_path / "fakebin" - fake_bin.mkdir() - fake_git = fake_bin / "git" - fake_git.write_text("#!/bin/sh\nsleep 30\n", encoding="utf-8") - fake_git.chmod(0o755) - - monkeypatch.setenv("PATH", os.pathsep.join([str(fake_bin), os.environ["PATH"]])) + _config(project) + _fake_git(tmp_path, "#!/bin/sh\nsleep 30\n", monkeypatch) monkeypatch.setattr(event_collector, "GIT_PROBE_TIMEOUT_SECONDS", 0.3) started = time.monotonic() @@ -101,10 +112,49 @@ def test_a_git_that_never_answers_stays_inside_the_budget( assert elapsed < 5.0, f"the probe held the command open for {elapsed:.2f}s" +@pytest.mark.skipif(os.name != "posix", reason="core.fsmonitor needs a POSIX hook") +def test_a_timed_out_probe_takes_the_helpers_it_started( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Git starts an fsmonitor hook for `ls-files`, so the budget owns it too.""" + + project = _git_project(tmp_path) + _config(project) + subprocess.run(["git", "-C", str(project), "config", "user.email", "f@e.invalid"], check=True) + subprocess.run(["git", "-C", str(project), "config", "user.name", "Fixture"], check=True) + (project / "tracked.txt").write_text("original\n", encoding="utf-8") + subprocess.run(["git", "-C", str(project), "add", "tracked.txt"], check=True) + subprocess.run(["git", "-C", str(project), "commit", "-qm", "fixture"], check=True) + + heartbeats = tmp_path / "heartbeats" + heartbeats.write_text("", encoding="utf-8") + hook = project / ".git" / "blocking-fsmonitor" + hook.write_text( + "#!/bin/sh\n" + f"while :; do echo tick >> {shlex.quote(str(heartbeats))}; sleep 0.1; done\n", + encoding="utf-8", + ) + hook.chmod(0o755) + subprocess.run( + ["git", "-C", str(project), "config", "core.fsmonitor", str(hook)], check=True + ) + monkeypatch.setattr(event_collector, "GIT_PROBE_TIMEOUT_SECONDS", 0.8) + + started = time.monotonic() + with pytest.raises(ValueError, match="needs a Git answer"): + _project_config_path(project, ".loopx/config/collector.json") + assert time.monotonic() - started < 6 + + retired = heartbeats.stat().st_size + assert retired > 0, "the fixture hook never ran, so this proves nothing" + time.sleep(0.6) + assert heartbeats.stat().st_size == retired, ( + "the fsmonitor hook and its child outlived the probe budget" + ) + + def test_an_ignored_untracked_config_still_loads(tmp_path: Path) -> None: project = _git_project(tmp_path) - target = project / ".loopx" / "config" / "collector.json" - target.parent.mkdir(parents=True) - target.write_text("{}", encoding="utf-8") + target = _config(project) assert _project_config_path(project, ".loopx/config/collector.json") == target.resolve() From 9e4215e47e72865fe612c8db91a1089c96321c9d Mon Sep 17 00:00:00 2001 From: ChengLong GU <88359731+gcl-coder@users.noreply.github.com> Date: Fri, 6 Nov 2026 12:30:44 +0800 Subject: [PATCH 3/3] fix(lark): retire the probe's session when collector-plan is cancelled The budget covered an exhausted wait, not an interrupted one. The probe runs in a session of its own so an exhausted budget can retire the helpers Git started, which also means a terminal Ctrl-C -- a SIGINT to the process group of the running `collector-plan` -- never reaches Git. `communicate` raised `KeyboardInterrupt` out of the only place that owns that session, the command exited, and the blocked Git and its helper kept running. Repeated cancellations accumulate the same background work the budget fix removed. The wait now retires the owned session on any interrupted exit and re-raises the original interruption: an exhausted budget keeps refusing the config, while Ctrl-C stays Ctrl-C. Reproduced with the `collector-plan` planning entry in a real child process, real Git and a `git` that blocks the ownership read with a helper writing heartbeats: interrupting the process group left the heartbeat growing on the previous head and stops it now. Signed-off-by: ChengLong GU <88359731+gcl-coder@users.noreply.github.com> --- loopx/extensions/lark/event_collector.py | 11 ++- tests/test_lark_event_collector_git_budget.py | 85 +++++++++++++++++++ 2 files changed, 95 insertions(+), 1 deletion(-) diff --git a/loopx/extensions/lark/event_collector.py b/loopx/extensions/lark/event_collector.py index 3befdd967f..efed1c9e13 100644 --- a/loopx/extensions/lark/event_collector.py +++ b/loopx/extensions/lark/event_collector.py @@ -87,7 +87,10 @@ def _run_git_probe( config is ignored and untracked, so a probe that runs out of budget lands on the refusing side instead of raising out of the command. The three questions share one deadline, and a question asked after it has passed is - not started at all. + not started at all. The probe runs in a session of its own so an exhausted + budget can retire the helpers Git started; that also means cancelling the + command reaches this process's group and not the probe, so an interrupted + wait retires the same session before the cancellation keeps travelling. """ budget = ( @@ -115,6 +118,12 @@ def _run_git_probe( except subprocess.TimeoutExpired: _retire_owned_probe(process) return None + except BaseException: + # The operator cancelled the command. An exhausted budget keeps + # refusing the config, while Ctrl-C retires its helpers and stays the + # cancellation the caller raised. + _retire_owned_probe(process) + raise return subprocess.CompletedProcess( args=argv, returncode=process.returncode, stdout=stdout, stderr="" ) diff --git a/tests/test_lark_event_collector_git_budget.py b/tests/test_lark_event_collector_git_budget.py index ed4682bfaa..7628b71d6d 100644 --- a/tests/test_lark_event_collector_git_budget.py +++ b/tests/test_lark_event_collector_git_budget.py @@ -10,7 +10,10 @@ import os import shlex +import shutil +import signal import subprocess +import sys import time from pathlib import Path @@ -158,3 +161,85 @@ def test_an_ignored_untracked_config_still_loads(tmp_path: Path) -> None: target = _config(project) assert _project_config_path(project, ".loopx/config/collector.json") == target.resolve() + + +@pytest.mark.skipif(os.name != "posix", reason="a terminal Ctrl-C is a POSIX group signal") +def test_a_cancelled_command_takes_the_git_it_started(tmp_path: Path) -> None: + """Ctrl-C during the admission retires the session the read owns. + + `collector-plan` dispatches to `plan_lark_event_collector`, so the child + below runs that entry in its own process, as the CLI would, with a `git` + that answers everything through real Git except the ownership read: that + one blocks, the shape a real `core.fsmonitor` hook has while Git waits on + it. The interrupt then goes to the child's own process group, which is the + scope a terminal Ctrl-C reaches. Git runs in a session of its own, so the + interrupt never reaches it: only the waiting call can retire it. + """ + + project = _git_project(tmp_path) + _config(project) + real_git = shutil.which("git") + assert real_git is not None + heartbeats = tmp_path / "heartbeats" + heartbeats.write_text("", encoding="utf-8") + stop = tmp_path / "stop" + bin_dir = tmp_path / "blocking-bin" + bin_dir.mkdir() + fake = bin_dir / "git" + fake.write_text( + f"#!{sys.executable}\n" + "import os, sys, time\n" + "from pathlib import Path\n" + "if 'ls-files' not in sys.argv:\n" + f" os.execv({real_git!r}, [{real_git!r}] + sys.argv[1:])\n" + f"heartbeats, stop = Path({str(heartbeats)!r}), Path({str(stop)!r})\n" + "while not stop.exists():\n" + " with heartbeats.open('a', encoding='utf-8') as stream:\n" + " stream.write('tick\\n')\n" + " time.sleep(0.1)\n", + encoding="utf-8", + ) + fake.chmod(0o755) + environment = { + key: value + for key, value in os.environ.items() + if not key.startswith(("PYTEST", "COVERAGE", "COV_CORE")) + } + environment.update({ + "PYTHONPATH": str(Path(__file__).resolve().parents[1]), + "LOOPX_USAGE_PING": "0", + "PATH": f"{bin_dir}{os.pathsep}{os.environ['PATH']}", + }) + command = subprocess.Popen( + [ + sys.executable, "-c", + "import sys\n" + "from loopx.extensions.lark.event_collector import plan_lark_event_collector\n" + "plan_lark_event_collector(project=sys.argv[1], config_path=sys.argv[2])\n", + str(project), ".loopx/config/collector.json", + ], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + env=environment, + start_new_session=True, + ) + try: + deadline = time.monotonic() + 25 + while heartbeats.stat().st_size == 0 and time.monotonic() < deadline: + time.sleep(0.05) + assert heartbeats.stat().st_size > 0, "the fixture never blocked, so this proves nothing" + assert command.poll() is None, "the command had already returned" + os.killpg(os.getpgid(command.pid), signal.SIGINT) + command.communicate(timeout=25) + assert command.returncode != 0 + retired = heartbeats.stat().st_size + time.sleep(0.6) + assert heartbeats.stat().st_size == retired, ( + "the blocked Git and its helper outlived the cancellation" + ) + finally: + stop.write_text("stop\n", encoding="utf-8") + if command.poll() is None: + os.killpg(os.getpgid(command.pid), signal.SIGKILL) + command.communicate(timeout=25)