diff --git a/loopx/contract.py b/loopx/contract.py index ac032e5d8d..b310687f6e 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, @@ -319,54 +320,135 @@ def _credential_hits_are_all_references(line: str) -> bool: TERMINAL_TODO_STATUSES = {TODO_STATUS_DONE, TODO_STATUS_DEFERRED, "completed", "closed", "archived"} -def _git_worktree_root(path: Path) -> Path | None: - path = path.resolve() - target = path if path.is_dir() else path.parent +# 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. 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 + + +class GitProbeUnavailable(RuntimeError): + """Git did not answer an ownership question inside the scan budget. + + 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. 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] try: - root = subprocess.run( - ["git", "-C", str(target), "rev-parse", "--show-toplevel"], + process = subprocess.Popen( + argv, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, text=True, encoding="utf-8", errors="replace", - capture_output=True, - check=True, - ).stdout.strip() - except (subprocess.CalledProcessError, FileNotFoundError): + start_new_session=os.name == "posix", + ) + except OSError: + return None + try: + stdout, _stderr = process.communicate(timeout=GIT_PROBE_TIMEOUT_SECONDS) + 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) + + +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 + 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 = ( - 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 - ) - 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 - ) + + 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), @@ -941,6 +1023,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: @@ -952,14 +1041,9 @@ 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, - ) + 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 [] @@ -1063,14 +1147,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) @@ -1126,6 +1220,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] = {} @@ -1153,16 +1248,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): @@ -1180,10 +1284,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"): @@ -1262,7 +1381,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), @@ -1275,6 +1400,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, @@ -1456,16 +1584,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: @@ -1483,6 +1618,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 new file mode 100644 index 0000000000..8872f1d060 --- /dev/null +++ b/tests/test_contract_git_probe_budget.py @@ -0,0 +1,192 @@ +"""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 time +from pathlib import Path + +import pytest + +from loopx import contract + + +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_an_unanswered_ownership_question_is_not_a_negative_answer( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + target = tmp_path / "public.md" + target.write_text("public", encoding="utf-8") + asked: list[tuple[str, ...]] = [] + + 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, "_git_query", fake_query) + observed = contract._git_probe(target) + + 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 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: + 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) + + started = time.monotonic() + boundary = contract.scan_public_boundary([repo]) + elapsed = time.monotonic() - started + + 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() + 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 boundary["ok"] is True + assert boundary["unavailable_git_probes"] == [] 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)