From 430863cf0edc51ffd1363b88e491985a5007e1f3 Mon Sep 17 00:00:00 2001 From: ChengLong GU <88359731+gcl-coder@users.noreply.github.com> Date: Thu, 5 Nov 2026 22:50:21 +0800 Subject: [PATCH 1/3] fix(contract): bound the Git ownership probes in a public-boundary scan A scan reads a checkout the operator may not control, and its four Git calls passed no budget at all: `rev-parse`, the `ls-files --error-unmatch` probe, `check-ignore`, and `ls-files -z`. A Git blocked on a stale index lock, a network mount or an oversized index therefore held the whole scan open, with no output and no way to tell the caller anything was wrong. Reproduced here with a fake `git` on PATH that only sleeps: `_git_probe` was still running after 5s and `scan_public_boundary` after 6s. Both calls are now bounded by `GIT_PROBE_TIMEOUT_SECONDS`. The budget needed the degradation paths, not just a `timeout` argument: `contract.py` handled no `TimeoutExpired` at all, so a bare timeout would have turned a hang into a traceback out of a read-only scan. A probe that runs out of budget now answers the same way a missing Git does -- ownership unknown -- so callers keep their conservative answer. The queries are local reads that never fan out into helpers, so the leader-only kill that `subprocess.run` performs covers them; the process tree retirement that `loopx/registry.py` needs does not apply here. Signed-off-by: ChengLong GU <88359731+gcl-coder@users.noreply.github.com> --- loopx/contract.py | 76 +++++++++++++++---------- tests/test_contract_git_probe_budget.py | 68 ++++++++++++++++++++++ 2 files changed, 115 insertions(+), 29 deletions(-) create mode 100644 tests/test_contract_git_probe_budget.py diff --git a/loopx/contract.py b/loopx/contract.py index ac032e5d8d..0bc1050419 100644 --- a/loopx/contract.py +++ b/loopx/contract.py @@ -319,6 +319,35 @@ def _credential_hits_are_all_references(line: str) -> bool: TERMINAL_TODO_STATUSES = {TODO_STATUS_DONE, TODO_STATUS_DEFERRED, "completed", "closed", "archived"} +# A scan reads a checkout the operator may not control, so a Git that blocks on +# a stale index lock, a network mount or an oversized index must not hold the +# scan open. These queries are local reads that never fan out into helpers. +GIT_PROBE_TIMEOUT_SECONDS = 5.0 + + +def _git_query_succeeds(*arguments: str) -> bool: + """Answer a Git ownership question, degrading when Git cannot answer. + + A probe that runs out of budget is treated like one that exited non-zero: + ownership stays unknown, which keeps the caller's conservative answer + instead of raising out of a read-only scan. + """ + + try: + completed = subprocess.run( + ["git", *arguments], + text=True, + encoding="utf-8", + errors="replace", + capture_output=True, + check=False, + timeout=GIT_PROBE_TIMEOUT_SECONDS, + ) + except (OSError, subprocess.SubprocessError): + return False + return completed.returncode == 0 + + def _git_worktree_root(path: Path) -> Path | None: path = path.resolve() target = path if path.is_dir() else path.parent @@ -330,8 +359,9 @@ def _git_worktree_root(path: Path) -> Path | None: errors="replace", capture_output=True, check=True, + timeout=GIT_PROBE_TIMEOUT_SECONDS, ).stdout.strip() - except (subprocess.CalledProcessError, FileNotFoundError): + except (subprocess.CalledProcessError, FileNotFoundError, subprocess.TimeoutExpired): return None return Path(root).resolve() @@ -345,27 +375,11 @@ def _git_probe(path: Path) -> dict[str, Any]: rel_path = str(path.relative_to(repo_root)) except ValueError: rel_path = str(path) - tracked = ( - subprocess.run( - ["git", "-C", str(repo_root), "ls-files", "--error-unmatch", "--", rel_path], - text=True, - encoding="utf-8", - errors="replace", - capture_output=True, - check=False, - ).returncode - == 0 + tracked = _git_query_succeeds( + "-C", str(repo_root), "ls-files", "--error-unmatch", "--", rel_path ) - ignored = ( - subprocess.run( - ["git", "-C", str(repo_root), "check-ignore", "-q", "--", rel_path], - text=True, - encoding="utf-8", - errors="replace", - capture_output=True, - check=False, - ).returncode - == 0 + ignored = _git_query_succeeds( + "-C", str(repo_root), "check-ignore", "-q", "--", rel_path ) return { "inside_worktree": True, @@ -952,14 +966,18 @@ def _tracked_scan_files(scan_root: Path) -> list[Path]: if not rel_root: rel_root = "." - tracked = subprocess.run( - ["git", "-C", str(repo_root), "ls-files", "-z", "--", rel_root], - text=True, - encoding="utf-8", - errors="replace", - capture_output=True, - check=False, - ) + try: + tracked = subprocess.run( + ["git", "-C", str(repo_root), "ls-files", "-z", "--", rel_root], + text=True, + encoding="utf-8", + errors="replace", + capture_output=True, + check=False, + timeout=GIT_PROBE_TIMEOUT_SECONDS, + ) + except (OSError, subprocess.SubprocessError): + return [] if tracked.returncode != 0: return [] diff --git a/tests/test_contract_git_probe_budget.py b/tests/test_contract_git_probe_budget.py new file mode 100644 index 0000000000..af6a7307fe --- /dev/null +++ b/tests/test_contract_git_probe_budget.py @@ -0,0 +1,68 @@ +from __future__ import annotations + +import os +import subprocess +import sys +import time +from pathlib import Path + +import pytest + +from loopx import contract + + +def _write_fake_git(directory: Path, script: str) -> None: + git = directory / "git" + git.write_text(script, encoding="utf-8") + git.chmod(0o755) + + +def test_ownership_questions_degrade_when_git_runs_out_of_budget( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A blocked Git leaves ownership unknown instead of raising out of a scan.""" + + target = tmp_path / "public.md" + target.write_text("public", encoding="utf-8") + + def fake_run(command, **kwargs): + if "rev-parse" in command: + return subprocess.CompletedProcess(command, 0, str(tmp_path), "") + raise subprocess.TimeoutExpired(command, kwargs.get("timeout")) + + monkeypatch.setattr(contract.subprocess, "run", fake_run) + observed = contract._git_probe(target) + + # 超时后既不能说它被跟踪，也不能说它被忽略：两条都落到最保守的答案。 + assert observed == { + "inside_worktree": True, + "root": str(tmp_path), + "tracked": False, + "ignored": False, + } + + +@pytest.mark.skipif(os.name != "posix", reason="the fake git is a POSIX shell script") +def test_probe_and_scan_finish_when_git_blocks( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A Git that never returns must not hold the read-only scan open.""" + + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + _write_fake_git(bin_dir, f"#!{sys.executable}\nimport time\ntime.sleep(30)\n") + scan_root = tmp_path / "public" + scan_root.mkdir() + (scan_root / "notes.md").write_text("public text", encoding="utf-8") + + monkeypatch.setenv("PATH", f"{bin_dir}{os.pathsep}{os.environ['PATH']}") + monkeypatch.setattr(contract, "GIT_PROBE_TIMEOUT_SECONDS", 0.3) + + started = time.monotonic() + assert contract._git_worktree_root(scan_root) is None + assert contract._git_query_succeeds("-C", str(scan_root), "ls-files") is False + result = contract.scan_public_boundary([scan_root]) + elapsed = time.monotonic() - started + + assert result["files"] >= 1 + assert elapsed < 20, f"a blocked Git held the scan for {elapsed:.1f}s" From 91c2cc681085b4f59ebbfc760a0d88d1995e0ddc Mon Sep 17 00:00:00 2001 From: ChengLong GU <88359731+gcl-coder@users.noreply.github.com> Date: Fri, 6 Nov 2026 10:18:26 +0800 Subject: [PATCH 2/3] fix(contract): keep an unanswered Git probe out of a clean scan answer Two gaps in the five-second budget. A probe that ran out of budget answered False, and False is the scan's word for "not tracked": a tracked ACTIVE_GOAL_STATE.md was skipped as private state and `check --scan-root` exited 0 with a clean boundary. `_tracked_scan_files` had the same shape, where an empty list hid tracked files behind a pruned directory. Silence is not a negative answer, so `_git_worktree_root` raises GitProbeUnavailable when Git cannot answer at all, `_git_probe` reports `tracked: None`, and a scan that hit one records it in `unavailable_git_probes`, which fails `ok` and produces the `public_boundary_scan_git_unavailable` diagnostic. Known-untracked private state is still excluded before it is read, and the first unanswered question ends the probing instead of spending another budget on the same hang. `subprocess.run(timeout=...)` retired the Git leader only, while Git starts an fsmonitor hook for `ls-files` that outlived it. 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 rather than draining them. Signed-off-by: ChengLong GU <88359731+gcl-coder@users.noreply.github.com> --- loopx/contract.py | 219 ++++++++++++++++++------ tests/test_contract_git_probe_budget.py | 198 +++++++++++++++++---- 2 files changed, 328 insertions(+), 89 deletions(-) diff --git a/loopx/contract.py b/loopx/contract.py index 0bc1050419..c2020c6030 100644 --- a/loopx/contract.py +++ b/loopx/contract.py @@ -36,6 +36,7 @@ from .control_plane.todos.active_state_editing import COMPLETED_WORK_ARCHIVE_HEADING from .control_plane.todos.active_state_todo_parser import active_state_incomplete_warning from .control_plane.todos.authoring_scope import todo_contract_diagnostics +from .extensions.process_runtime import terminate_process_tree from .history import ( RunHistoryAudit, build_run_history_audit, @@ -321,66 +322,125 @@ def _credential_hits_are_all_references(line: str) -> bool: # A scan reads a checkout the operator may not control, so a Git that blocks on # a stale index lock, a network mount or an oversized index must not hold the -# scan open. These queries are local reads that never fan out into helpers. +# scan open. Git runs helpers of its own -- an fsmonitor hook is the common one +# on `ls-files` -- so a budget that stops the caller has to stop that work too. GIT_PROBE_TIMEOUT_SECONDS = 5.0 -def _git_query_succeeds(*arguments: str) -> bool: - """Answer a Git ownership question, degrading when Git cannot answer. +class GitProbeUnavailable(RuntimeError): + """Git did not answer an ownership question inside the scan budget. - A probe that runs out of budget is treated like one that exited non-zero: - ownership stays unknown, which keeps the caller's conservative answer - instead of raising out of a read-only scan. + A missing answer is not a negative one. Callers of this module decide + whether an unanswered question lets them pass; they cannot read it as + "not tracked", because a tracked private file would then be published. """ + +@dataclass(frozen=True) +class _GitAnswer: + returncode: int + stdout: str + + +def _retire_owned_probe(process: subprocess.Popen[str]) -> None: + """Stop this probe's own process group, then drop its pipes unread. + + A descendant that inherited the pipes keeps them open after the leader + dies, so draining them would wait on work this call no longer owns. + """ + + 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_query(*arguments: str) -> _GitAnswer | None: + """One bounded local Git read, or None when it cannot answer in budget. + + Git is started in a session of its own so an exhausted budget retires the + helpers that read would have started along with it. + """ + + argv = ["git", *arguments] try: - completed = subprocess.run( - ["git", *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): - return False - return completed.returncode == 0 + except OSError: + return None + try: + stdout, _stderr = process.communicate(timeout=GIT_PROBE_TIMEOUT_SECONDS) + except subprocess.TimeoutExpired: + _retire_owned_probe(process) + return None + return _GitAnswer(returncode=process.returncode, stdout=stdout) def _git_worktree_root(path: Path) -> Path | None: + """Return the worktree root, or None when Git says this is not one. + + Raises :class:`GitProbeUnavailable` when Git could not answer at all, so + callers cannot mistake silence for "no repository here". + """ + path = path.resolve() target = path if path.is_dir() else path.parent - try: - root = subprocess.run( - ["git", "-C", str(target), "rev-parse", "--show-toplevel"], - text=True, - encoding="utf-8", - errors="replace", - capture_output=True, - check=True, - timeout=GIT_PROBE_TIMEOUT_SECONDS, - ).stdout.strip() - except (subprocess.CalledProcessError, FileNotFoundError, subprocess.TimeoutExpired): + answer = _git_query("-C", str(target), "rev-parse", "--show-toplevel") + if answer is None: + raise GitProbeUnavailable("git did not answer the worktree probe") + if answer.returncode != 0: + return None + root = answer.stdout.strip() + if not root: return None return Path(root).resolve() def _git_probe(path: Path) -> dict[str, Any]: + """Report Git ownership of one path; ``None`` marks a question Git did not answer.""" + path = path.resolve() - repo_root = _git_worktree_root(path) + try: + repo_root = _git_worktree_root(path) + except GitProbeUnavailable: + return {"inside_worktree": None, "tracked": None, "ignored": None} if repo_root is None: return {"inside_worktree": False, "tracked": False, "ignored": False} try: rel_path = str(path.relative_to(repo_root)) except ValueError: rel_path = str(path) - tracked = _git_query_succeeds( - "-C", str(repo_root), "ls-files", "--error-unmatch", "--", rel_path - ) - ignored = _git_query_succeeds( - "-C", str(repo_root), "check-ignore", "-q", "--", rel_path - ) + + def _ask(*arguments: str) -> bool | None: + answer = _git_query(*arguments) + return None if answer is None else answer.returncode == 0 + + tracked = _ask("-C", str(repo_root), "ls-files", "--error-unmatch", "--", rel_path) + if tracked is None: + # Ownership is already unproven, and no caller can act on the ignore + # flag of a file whose ownership is unknown; the second read is wasted. + return { + "inside_worktree": True, + "root": str(repo_root), + "tracked": None, + "ignored": None, + } + ignored = _ask("-C", str(repo_root), "check-ignore", "-q", "--", rel_path) return { "inside_worktree": True, "root": str(repo_root), @@ -955,6 +1015,13 @@ def _authority_source_diagnostics( def _tracked_scan_files(scan_root: Path) -> list[Path]: + """Enumerate the Git-owned files under one scan root. + + Raises :class:`GitProbeUnavailable` when Git did not answer: an incomplete + enumeration would silently drop the files a pruned directory hides, which + is exactly what the caller cannot afford to miss. + """ + scan_root = scan_root.resolve() repo_root = _git_worktree_root(scan_root) if repo_root is None: @@ -966,18 +1033,9 @@ def _tracked_scan_files(scan_root: Path) -> list[Path]: if not rel_root: rel_root = "." - try: - tracked = subprocess.run( - ["git", "-C", str(repo_root), "ls-files", "-z", "--", rel_root], - text=True, - encoding="utf-8", - errors="replace", - capture_output=True, - check=False, - timeout=GIT_PROBE_TIMEOUT_SECONDS, - ) - except (OSError, subprocess.SubprocessError): - return [] + tracked = _git_query("-C", str(repo_root), "ls-files", "-z", "--", rel_root) + if tracked is None: + raise GitProbeUnavailable("git did not answer the tracked-file enumeration") if tracked.returncode != 0: return [] @@ -1081,14 +1139,24 @@ def _probe_directory_listing( def iter_scan_files( - scan_root: Path, *, unreadable_directories: list[tuple[Path, str]] | None = None + scan_root: Path, + *, + unreadable_directories: list[tuple[Path, str]] | None = None, + unavailable_git_probes: list[str] | None = None, ) -> list[Path]: scan_root = scan_root.resolve() if scan_root.is_file(): return [scan_root] files: list[Path] = [] unreadable = unreadable_directories if unreadable_directories is not None else [] - tracked_files = _tracked_scan_files(scan_root) + try: + tracked_files = _tracked_scan_files(scan_root) + except GitProbeUnavailable: + # Walk what is reachable, but remember that the Git side of the + # enumeration is missing so the scan cannot be called clean. + if unavailable_git_probes is not None: + unavailable_git_probes.append(str(scan_root)) + tracked_files = [] # The root is probed even when dependency pruning skips the walk, so an # unreadable scan target cannot pass because of how it was named. _probe_directory_listing(scan_root, unreadable) @@ -1144,6 +1212,7 @@ def scan_public_boundary( credential_reference_hits: list[str] = [] unreadable_files: list[str] = [] unreadable_directories: list[tuple[Path, str]] = [] + unavailable_git_probes: list[str] = [] missing_scan_roots: list[str] = [] files: list[Path] = [] file_roots: dict[Path, Path] = {} @@ -1171,16 +1240,25 @@ def record_hit(hit: str, *, blocking: bool = True) -> None: # A narrow explicit input cannot erase private ancestry. A real # worktree is its own namespace, even beneath a private parent; # outside Git, retain the complete canonical ancestry. - classification_root = ( - _git_worktree_root(resolved_scan_root) or Path(resolved_scan_root.anchor) - ) + try: + classification_root = ( + _git_worktree_root(resolved_scan_root) + or Path(resolved_scan_root.anchor) + ) + except GitProbeUnavailable: + # The canonical ancestry is the wider answer: it can only add + # private state, never hide it. + classification_root = Path(resolved_scan_root.anchor) + unavailable_git_probes.append(str(resolved_scan_root)) if _is_local_private_state_path(resolved_scan_root, classification_root): # Explicit outgoing selection cannot turn an ignored private file # or a pruned private directory into a successful empty scan. record_hit(f"{rel_or_abs(resolved_scan_root, classification_root)}: local_private_state") continue for file_path in iter_scan_files( - resolved_scan_root, unreadable_directories=unreadable_directories + resolved_scan_root, + unreadable_directories=unreadable_directories, + unavailable_git_probes=unavailable_git_probes, ): files.append(file_path) if _is_local_private_state_path(file_path, classification_root): @@ -1198,10 +1276,25 @@ def public_files() -> Iterator[Path]: # Filtering precedes submission: a worker must never open untracked # local-private state. Policy, Git ownership and content classification # stay here, in the existing scan owner, not in the I/O adapter. + ownership_unavailable = False for path in files: root = file_roots.get(path, path) if path in private_files: + if ownership_unavailable: + # One unanswered question already decides the scan; asking + # again would only spend another budget on the same hang. + unavailable_git_probes.append(rel_or_abs(path, root)) + skipped_private_state_files.append(rel_or_abs(path, root)) + continue git = _git_probe(path) + if git.get("tracked") is None: + ownership_unavailable = True + # Git did not answer, so ownership is unproven. Private + # state is still never opened, and the scan stops asking + # once one answer is missing. + unavailable_git_probes.append(rel_or_abs(path, root)) + skipped_private_state_files.append(rel_or_abs(path, root)) + continue if not git.get("tracked"): skipped_private_state_files.append(rel_or_abs(path, root)) if git.get("inside_worktree") and not git.get("ignored"): @@ -1280,7 +1373,13 @@ def public_files() -> Iterator[Path]: else: record_hit(hit, blocking=rule.blocking) return { - "ok": not (blocking_hits or missing_scan_roots or unreadable_files or unreadable_directories), + "ok": not ( + blocking_hits + or missing_scan_roots + or unreadable_files + or unreadable_directories + or unavailable_git_probes + ), "scan_roots": [str(path) for path in scan_roots], "files": len(files), "scanned_files": len(files) - len(skipped_private_state_files), @@ -1293,6 +1392,9 @@ def public_files() -> Iterator[Path]: _unreadable_directory_labels(unreadable_directories, directory_roots) ), "missing_scan_roots": missing_scan_roots, + # A Git question that was not answered leaves the boundary unknown: + # this list keeps a partial answer from being reported as clean. + "unavailable_git_probes": sorted(set(unavailable_git_probes)), "allowed_hits": allowed_hits, "private_state_git_warnings": private_state_git_warnings, "policy": policy, @@ -1474,16 +1576,23 @@ def add_global_error(code: str, message: str) -> None: str(item) for item in boundary.get("unreadable_directories") or [] ] unreadable_files = [str(item) for item in boundary.get("unreadable_files") or []] + unavailable_git_probes = [ + str(item) for item in boundary.get("unavailable_git_probes") or [] + ] boundary_warnings = [str(item) for item in boundary.get("warnings") or []] public_boundary_scan = { "state": "completed", "ok": bool(boundary.get("ok")) and not ( - missing_scan_roots or unreadable_directories or unreadable_files + missing_scan_roots + or unreadable_directories + or unreadable_files + or unavailable_git_probes ), "scanned_files": int(boundary.get("scanned_files") or 0), "missing_scan_roots": missing_scan_roots, "unreadable_directories": unreadable_directories, "unreadable_files": unreadable_files, + "unavailable_git_probes": unavailable_git_probes, "warnings": boundary_warnings, } for missing_root in missing_scan_roots: @@ -1501,6 +1610,12 @@ def add_global_error(code: str, message: str) -> None: "public_boundary_scan_file_unreadable", f"scan file could not be read: {unreadable_file}", ) + for unavailable_probe in unavailable_git_probes: + add_global_error( + "public_boundary_scan_git_unavailable", + "Git could not answer ownership for: " + f"{unavailable_probe}", + ) if public_boundary_scan["ok"]: label = "completed with warnings" if boundary_warnings else "clean" checks.append(f"public boundary scan {label}: {boundary.get('scanned_files')} files") diff --git a/tests/test_contract_git_probe_budget.py b/tests/test_contract_git_probe_budget.py index af6a7307fe..8872f1d060 100644 --- a/tests/test_contract_git_probe_budget.py +++ b/tests/test_contract_git_probe_budget.py @@ -1,8 +1,17 @@ +"""A Git probe that runs out of budget must not turn into a clean answer. + +The scan decides whether a tracked private file is published. "Git says it is +not in the index" and "Git did not answer" therefore have to stay different +answers, and the process tree a blocked probe started has to stop with it. +""" + from __future__ import annotations +import json import os +import shlex +import shutil import subprocess -import sys import time from pathlib import Path @@ -11,58 +20,173 @@ from loopx import contract -def _write_fake_git(directory: Path, script: str) -> None: - git = directory / "git" - git.write_text(script, encoding="utf-8") - git.chmod(0o755) +def _registry(tmp_path: Path) -> Path: + registry_path = tmp_path / "registry.json" + registry_path.write_text( + json.dumps( + { + "schema_version": 1, + "common_runtime_root": str(tmp_path / "runtime"), + "goals": [], + } + ), + encoding="utf-8", + ) + return registry_path + + +def _repository(tmp_path: Path) -> Path: + repo = tmp_path / "repo" + repo.mkdir() + subprocess.run(["git", "init", "-q", str(repo)], check=True) + subprocess.run(["git", "-C", str(repo), "config", "user.email", "f@e.invalid"], check=True) + subprocess.run(["git", "-C", str(repo), "config", "user.name", "Fixture"], check=True) + (repo / "ACTIVE_GOAL_STATE.md").write_text("# tracked private state\n", encoding="utf-8") + (repo / "NOTES.md").write_text("# public notes\n", encoding="utf-8") + subprocess.run( + ["git", "-C", str(repo), "add", "ACTIVE_GOAL_STATE.md", "NOTES.md"], check=True + ) + subprocess.run(["git", "-C", str(repo), "commit", "-qm", "fixture"], check=True) + return repo + + +def _delay_git( + tmp_path: Path, needle: str, monkeypatch: pytest.MonkeyPatch +) -> None: + """Put a `git` on PATH that waits forever on the query holding ``needle``.""" + + real_git = shutil.which("git") + assert real_git is not None + bin_dir = tmp_path / "slow-bin" + bin_dir.mkdir(exist_ok=True) + wrapper = bin_dir / "git" + wrapper.write_text( + "#!/bin/sh\n" + 'for argument in "$@"; do\n' + f' case "$argument" in *{needle}*) sleep 30 ;; esac\n' + "done\n" + f"exec {shlex.quote(real_git)} \"$@\"\n", + encoding="utf-8", + ) + wrapper.chmod(0o755) + monkeypatch.setenv("PATH", f"{bin_dir}{os.pathsep}{os.environ['PATH']}") -def test_ownership_questions_degrade_when_git_runs_out_of_budget( +def test_an_unanswered_ownership_question_is_not_a_negative_answer( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: - """A blocked Git leaves ownership unknown instead of raising out of a scan.""" - target = tmp_path / "public.md" target.write_text("public", encoding="utf-8") + asked: list[tuple[str, ...]] = [] - def fake_run(command, **kwargs): - if "rev-parse" in command: - return subprocess.CompletedProcess(command, 0, str(tmp_path), "") - raise subprocess.TimeoutExpired(command, kwargs.get("timeout")) + def fake_query(*arguments: str) -> contract._GitAnswer | None: + asked.append(arguments) + if "rev-parse" in arguments: + return contract._GitAnswer(returncode=0, stdout=f"{tmp_path}\n") + return None - monkeypatch.setattr(contract.subprocess, "run", fake_run) + monkeypatch.setattr(contract, "_git_query", fake_query) observed = contract._git_probe(target) - # 超时后既不能说它被跟踪，也不能说它被忽略：两条都落到最保守的答案。 - assert observed == { - "inside_worktree": True, - "root": str(tmp_path), - "tracked": False, - "ignored": False, - } + assert observed["tracked"] is None + assert observed["ignored"] is None + # The first unanswered question settles it; the ignore flag of a file whose + # ownership is unknown cannot change any caller's answer. + assert [arguments[2] for arguments in asked] == ["rev-parse", "ls-files"] -@pytest.mark.skipif(os.name != "posix", reason="the fake git is a POSIX shell script") -def test_probe_and_scan_finish_when_git_blocks( +@pytest.mark.skipif(os.name != "posix", reason="the delayed git is a POSIX script") +def test_tracked_private_state_stays_blocked_when_the_track_query_times_out( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: - """A Git that never returns must not hold the read-only scan open.""" + repo = _repository(tmp_path) + _delay_git(tmp_path, "--error-unmatch", monkeypatch) + # The budget has to be well above a real Git start-up, while the delayed + # query still spends far longer than the test is willing to wait. + monkeypatch.setattr(contract, "GIT_PROBE_TIMEOUT_SECONDS", 1.5) + + boundary = contract.scan_public_boundary([repo]) + + assert boundary["ok"] is False + assert "ACTIVE_GOAL_STATE.md" in boundary["unavailable_git_probes"] + # Private state is still never opened while ownership is unproven. + assert boundary["skipped_private_state_files"] == ["ACTIVE_GOAL_STATE.md"] + + checked = contract.check_contract( + registry_path=_registry(tmp_path), + runtime_root_override=None, + scan_roots=[repo], + limit=5, + ) + assert checked["ok"] is False + assert checked["public_boundary_scan"]["ok"] is False + assert "ACTIVE_GOAL_STATE.md" in checked["public_boundary_scan"][ + "unavailable_git_probes" + ] + codes = [diagnostic["code"] for diagnostic in checked["error_diagnostics"]] + assert "public_boundary_scan_git_unavailable" in codes + assert not [ + check for check in checked["checks"] if check.startswith("public boundary scan clean") + ] + + +@pytest.mark.skipif(os.name != "posix", reason="the delayed git is a POSIX script") +def test_an_incomplete_enumeration_is_not_a_complete_scan( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + repo = _repository(tmp_path) + _delay_git(tmp_path, "-z", monkeypatch) + monkeypatch.setattr(contract, "GIT_PROBE_TIMEOUT_SECONDS", 1.5) - bin_dir = tmp_path / "bin" - bin_dir.mkdir() - _write_fake_git(bin_dir, f"#!{sys.executable}\nimport time\ntime.sleep(30)\n") - scan_root = tmp_path / "public" - scan_root.mkdir() - (scan_root / "notes.md").write_text("public text", encoding="utf-8") + started = time.monotonic() + boundary = contract.scan_public_boundary([repo]) + elapsed = time.monotonic() - started - monkeypatch.setenv("PATH", f"{bin_dir}{os.pathsep}{os.environ['PATH']}") - monkeypatch.setattr(contract, "GIT_PROBE_TIMEOUT_SECONDS", 0.3) + assert boundary["ok"] is False + assert str(repo) in boundary["unavailable_git_probes"] + assert elapsed < 20, f"a blocked enumeration held the scan for {elapsed:.1f}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.""" + + repo = _repository(tmp_path) + heartbeats = tmp_path / "heartbeats" + heartbeats.write_text("", encoding="utf-8") + hook = repo / ".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(repo), "config", "core.fsmonitor", str(hook)], check=True + ) + monkeypatch.setattr(contract, "GIT_PROBE_TIMEOUT_SECONDS", 0.8) started = time.monotonic() - assert contract._git_worktree_root(scan_root) is None - assert contract._git_query_succeeds("-C", str(scan_root), "ls-files") is False - result = contract.scan_public_boundary([scan_root]) - elapsed = time.monotonic() - started + with pytest.raises(contract.GitProbeUnavailable): + contract._tracked_scan_files(repo) + 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_a_non_git_directory_still_scans_cleanly(tmp_path: Path) -> None: + public = tmp_path / "public" + public.mkdir() + (public / "NOTES.md").write_text("# Public\n", encoding="utf-8") + + boundary = contract.scan_public_boundary([public]) - assert result["files"] >= 1 - assert elapsed < 20, f"a blocked Git held the scan for {elapsed:.1f}s" + assert boundary["ok"] is True + assert boundary["unavailable_git_probes"] == [] From 7462420d3872a825bc5c8dc4ffe0a8ba26a56f8b Mon Sep 17 00:00:00 2001 From: ChengLong GU <88359731+gcl-coder@users.noreply.github.com> Date: Fri, 6 Nov 2026 21:11:57 +0800 Subject: [PATCH 3/3] fix(contract): retire the probe's session when the scan is cancelled The probe runs in a session of its own so an exhausted budget can retire the helpers Git started. That also means a terminal Ctrl-C -- a SIGINT to the process group of the running `check` -- never reaches Git, and `communicate` raised `KeyboardInterrupt` out of the only place that owned that session. The scan exited while the blocked Git, its fsmonitor hook and the hook's child kept running, so repeated cancellations accumulated work the budget fix removed. An interrupted wait now retires the owned session before the cancellation keeps travelling: an exhausted budget still answers "unavailable", while Ctrl-C stays Ctrl-C. Reproduced with real Git and a blocked `ls-files` behind a helper that writes heartbeats; interrupting the scan's process group stops it now. Signed-off-by: ChengLong GU <88359731+gcl-coder@users.noreply.github.com> --- loopx/contract.py | 10 ++- tests/test_contract_git_probe_cancel.py | 102 ++++++++++++++++++++++++ 2 files changed, 111 insertions(+), 1 deletion(-) create mode 100644 tests/test_contract_git_probe_cancel.py diff --git a/loopx/contract.py b/loopx/contract.py index c2020c6030..b310687f6e 100644 --- a/loopx/contract.py +++ b/loopx/contract.py @@ -367,7 +367,9 @@ def _git_query(*arguments: str) -> _GitAnswer | None: """One bounded local Git read, or None when it cannot answer in budget. Git is started in a session of its own so an exhausted budget retires the - helpers that read would have started along with it. + helpers that read would have started along with it. That session also keeps + a terminal Ctrl-C away from Git, so an interrupted wait retires the same + helpers before the cancellation keeps travelling. """ argv = ["git", *arguments] @@ -388,6 +390,12 @@ def _git_query(*arguments: str) -> _GitAnswer | None: except subprocess.TimeoutExpired: _retire_owned_probe(process) return None + except BaseException: + # The operator cancelled the scan. An exhausted budget keeps answering + # "unavailable", while Ctrl-C retires this read's helpers and stays the + # cancellation the caller raised. + _retire_owned_probe(process) + raise return _GitAnswer(returncode=process.returncode, stdout=stdout) diff --git a/tests/test_contract_git_probe_cancel.py b/tests/test_contract_git_probe_cancel.py new file mode 100644 index 0000000000..362fe809e5 --- /dev/null +++ b/tests/test_contract_git_probe_cancel.py @@ -0,0 +1,102 @@ +"""Cancelling the public-boundary scan must retire the Git read it started. + +The scan launches Git in a session of its own so an exhausted budget can retire +the helpers Git starts. That also keeps a terminal Ctrl-C away from Git: the +interrupt reaches the process group of the running `check`, so only the waiting +read can retire the session before the cancellation keeps travelling. +""" + +from __future__ import annotations + +import os +import shutil +import signal +import subprocess +import sys +import time +from pathlib import Path + +import pytest + +_RUN_SCAN = ( + "import sys\n" + "from pathlib import Path\n" + "from loopx.contract import scan_public_boundary\n" + "scan_public_boundary([Path(sys.argv[1])])\n" +) + + +def _repository(tmp_path: Path) -> Path: + repo = tmp_path / "repo" + repo.mkdir() + subprocess.run(["git", "init", "-q", str(repo)], check=True) + subprocess.run(["git", "-C", str(repo), "config", "user.email", "f@e.invalid"], check=True) + subprocess.run(["git", "-C", str(repo), "config", "user.name", "Fixture"], check=True) + (repo / "ACTIVE_GOAL_STATE.md").write_text("# tracked private state\n", encoding="utf-8") + subprocess.run(["git", "-C", str(repo), "add", "ACTIVE_GOAL_STATE.md"], check=True) + subprocess.run(["git", "-C", str(repo), "commit", "-qm", "fixture"], check=True) + return repo + + +@pytest.mark.skipif(os.name != "posix", reason="a terminal Ctrl-C is a POSIX group signal") +def test_a_cancelled_scan_takes_the_git_it_started(tmp_path: Path) -> None: + repo = _repository(tmp_path) + heartbeats = tmp_path / "heartbeats" + heartbeats.write_text("", encoding="utf-8") + stop = tmp_path / "stop" + real_git = shutil.which("git") + assert real_git is not None + 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", _RUN_SCAN, str(repo)], + 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 scan 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 probe 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)