diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 90e50d2..ece3ca6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -26,6 +26,9 @@ jobs: cache: 'pip' - name: Install run: python -m pip install -e ".[dev,tray]" + - name: Install zombie-killer-tray (Python 3.12+ only; it doesn't support 3.11) + if: matrix.python-version != '3.11' + run: python -m pip install "zombie-killer-tray @ git+https://github.com/dev-bricks/zombie-killer-tray.git@039b4f2c7acd69063b39d6168c757b0b2fca2438" - name: Lint run: ruff check . - name: Bytecode compilation check diff --git a/THIRD_PARTY_LICENSES.md b/THIRD_PARTY_LICENSES.md index 9319778..5145655 100644 --- a/THIRD_PARTY_LICENSES.md +++ b/THIRD_PARTY_LICENSES.md @@ -57,6 +57,8 @@ The core runtime of `safe-start-for-codex` has **zero external runtime dependenc | **pytest** | `>=9.1.1` | Automated test runner, contract verification suites, mock fixtures | [MIT](https://github.com/pytest-dev/pytest/blob/main/LICENSE) | [pytest-dev/pytest](https://github.com/pytest-dev/pytest) | | **ruff** | `>=0.5.0` | High-performance Python linter and code formatting enforcement | [MIT / Apache-2.0](https://github.com/astral-sh/ruff/blob/main/LICENSE-MIT) | [astral-sh/ruff](https://github.com/astral-sh/ruff) | | **PyInstaller** | `>=6.0` | Optional standalone Windows executable builder (`build_exe.bat`) | [GPLv2-or-later with Special Exception](https://github.com/pyinstaller/pyinstaller/blob/develop/COPYING.txt) | [pyinstaller/pyinstaller](https://github.com/pyinstaller/pyinstaller) | +| **zombie-killer-tray** | commit `039b4f2` | Optional, Windows-only companion for orphaned MCP/language-server process cleanup, launched as its own subprocess | [MIT](https://github.com/dev-bricks/zombie-killer-tray/blob/main/LICENSE) | [dev-bricks/zombie-killer-tray](https://github.com/dev-bricks/zombie-killer-tray) | +| **psutil** | `>=7.2,<8` | Watcher PID/create_time verification (Windows + Python>=3.12 only) | [BSD-3-Clause](https://github.com/giampaolo/psutil/blob/master/LICENSE) | [giampaolo/psutil](https://github.com/giampaolo/psutil) | --- diff --git a/THIRD_PARTY_LICENSES.txt b/THIRD_PARTY_LICENSES.txt index 3a075f7..7ff77b3 100644 --- a/THIRD_PARTY_LICENSES.txt +++ b/THIRD_PARTY_LICENSES.txt @@ -27,6 +27,8 @@ The base package declares no external runtime dependencies in | pytest | Development tests | `>=9.1.1` | 9.1.1 | MIT | https://pypi.org/project/pytest/ | | ruff | Development linter | `>=0.5.0` | 0.6.2 | MIT / Apache-2.0 | https://pypi.org/project/ruff/ | | PyInstaller | Optional Windows EXE build | `>=6.0` | 6.21.0 | GPLv2-or-later with PyInstaller's special exception | https://pypi.org/project/pyinstaller/ | +| zombie-killer-tray | Optional, Windows-only integration | commit `039b4f2c7acd69063b39d6168c757b0b2fca2438` | 0.1.0 | MIT | https://github.com/dev-bricks/zombie-killer-tray | +| psutil | Watcher PID/create_time verification, Windows/Python>=3.12 only | `>=7.2,<8` | 7.2.0 | BSD-3-Clause | https://pypi.org/project/psutil/ | ## Transitive Build & Test Inventory @@ -49,5 +51,8 @@ The base package declares no external runtime dependencies in - The optional `build` extra uses PyInstaller. PyInstaller metadata states a GPLv2-or-later license with a special exception that allows building and distributing non-free applications. +- The optional `zombie-killer` extra installs `zombie-killer-tray`, guarded by + a `sys_platform == 'win32'` environment marker so it is never attempted on + Linux/macOS even if that extra is requested there. - Binary release preparation should still inspect the actual bundled wheel set and include the license files from bundled distributions. diff --git a/pyproject.toml b/pyproject.toml index ba4b9c3..0e4b728 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -69,6 +69,22 @@ dev = [ build = [ "pyinstaller>=6.0" ] +# T-20260926-212716751: optional, Windows-only companion tool (conservative +# cleanup of orphaned MCP/language-server processes). Never installed +# implicitly -- only pulled in if a caller explicitly requests this extra. +# Two environment markers, combined: `sys_platform` protects `pip install +# .[zombie-killer]` on a non-Windows machine from failing on +# zombie-killer-tray's own Win32 deps; `python_version` does the same for +# Python 3.11 (this project's own floor is >=3.11, but zombie-killer-tray +# itself requires >=3.12 -- review finding: without this marker, the CI +# matrix's 3.11 leg would try and fail to install it). +zombie-killer = [ + "zombie-killer-tray @ git+https://github.com/dev-bricks/zombie-killer-tray.git@039b4f2c7acd69063b39d6168c757b0b2fca2438 ; sys_platform == 'win32' and python_version >= '3.12'", + # Declared explicitly (not just hoped-for transitively via + # zombie-killer-tray) -- stop_zombie_killer_watch()/launch's double-start + # guard fail-closed without it, so it must not be an implicit assumption. + "psutil>=7.2,<8 ; sys_platform == 'win32' and python_version >= '3.12'", +] [project.scripts] safe-start-for-codex = "safe_start_for_codex.cli:main" @@ -89,6 +105,12 @@ Notice = "https://github.com/dev-bricks/safe-start-for-codex/blob/main/NOTICE" "LLM Ready" = "https://github.com/dev-bricks/safe-start-for-codex/blob/main/llms.txt" +[tool.hatch.metadata] +# T-20260926-212716751: the optional `zombie-killer` extra declares a direct +# `git+https://...` reference (zombie-killer-tray), which hatchling refuses +# to build without this explicit opt-in (broke `pip install -e .` in CI). +allow-direct-references = true + [tool.hatch.build.targets.wheel] packages = ["src/safe_start_for_codex"] diff --git a/src/safe_start_for_codex/cli.py b/src/safe_start_for_codex/cli.py index c5d349c..2f54755 100644 --- a/src/safe_start_for_codex/cli.py +++ b/src/safe_start_for_codex/cli.py @@ -1730,6 +1730,57 @@ def setup(_icon: pystray.Icon) -> None: return 0 +def command_zombie_killer_report(args: argparse.Namespace) -> int: + from .zombie_killer_integration import build_zombie_killer_status + + status = build_zombie_killer_status() + if args.json: + print(json.dumps(status.to_dict(), ensure_ascii=False, indent=2)) + else: + print(status.to_text()) + return 0 + + +def command_zombie_killer_install(args: argparse.Namespace) -> int: + from .zombie_killer_integration import install_zombie_killer_package + + result = install_zombie_killer_package(target=args.target) + if args.json: + print(json.dumps(result.to_dict(), ensure_ascii=False, indent=2)) + else: + print(result.to_text()) + return 0 if result.status == "ok" else 1 + + +def command_zombie_killer_watch(args: argparse.Namespace) -> int: + from .zombie_killer_integration import ( + DEFAULT_MIN_AGE_SECONDS, + DEFAULT_WATCH_INTERVAL_SECONDS, + launch_zombie_killer_watch, + ) + + result = launch_zombie_killer_watch( + interval_seconds=args.interval or DEFAULT_WATCH_INTERVAL_SECONDS, + min_age_seconds=args.min_age or DEFAULT_MIN_AGE_SECONDS, + ) + if args.json: + print(json.dumps(result.to_dict(), ensure_ascii=False, indent=2)) + else: + print(result.to_text()) + return 0 if result.status == "ok" else 1 + + +def command_zombie_killer_stop(args: argparse.Namespace) -> int: + from .zombie_killer_integration import stop_zombie_killer_watch + + result = stop_zombie_killer_watch() + if args.json: + print(json.dumps(result.to_dict(), ensure_ascii=False, indent=2)) + else: + print(result.to_text()) + return 0 if result.status in ("ok", "not-running") else 1 + + def command_status(_: argparse.Namespace) -> int: latest = state_dir() / "latest.json" if not latest.exists(): @@ -1923,6 +1974,40 @@ def build_parser() -> argparse.ArgumentParser: backup = sub.add_parser("backup", help="Create a manual backup of automation TOML files.") backup.set_defaults(func=command_backup) + + zombie_killer_report = sub.add_parser( + "zombie-killer-report", + help="Check zombie-killer-tray status (last cleanup cycle). Optional, Windows-only integration.", + ) + zombie_killer_report.add_argument("--json", action="store_true") + zombie_killer_report.set_defaults(func=command_zombie_killer_report) + + zombie_killer_install = sub.add_parser( + "zombie-killer-install", + help="Install or upgrade zombie-killer-tray.", + ) + zombie_killer_install.add_argument( + "--target", default=None, + help="Optional pip target. Default: local sibling checkout, else the commit-pinned GitHub source.", + ) + zombie_killer_install.add_argument("--json", action="store_true") + zombie_killer_install.set_defaults(func=command_zombie_killer_install) + + zombie_killer_watch = sub.add_parser( + "zombie-killer-watch", + help="Launch zombie-killer-tray as its own watch subprocess (orphaned MCP/language-server processes). Windows-only.", + ) + zombie_killer_watch.add_argument("--interval", type=int, default=None) + zombie_killer_watch.add_argument("--min-age", type=int, default=None) + zombie_killer_watch.add_argument("--json", action="store_true") + zombie_killer_watch.set_defaults(func=command_zombie_killer_watch) + + zombie_killer_stop = sub.add_parser( + "zombie-killer-stop", + help="Stop a zombie-killer-tray watch subprocess started via zombie-killer-watch.", + ) + zombie_killer_stop.add_argument("--json", action="store_true") + zombie_killer_stop.set_defaults(func=command_zombie_killer_stop) return parser diff --git a/src/safe_start_for_codex/zombie_killer_integration.py b/src/safe_start_for_codex/zombie_killer_integration.py new file mode 100644 index 0000000..c56af05 --- /dev/null +++ b/src/safe_start_for_codex/zombie_killer_integration.py @@ -0,0 +1,541 @@ +"""Optional zombie-killer-tray integration for Safe Start (T-20260926-212716751). + +Safe Start for Codex is cross-platform (Windows/Linux/macOS) with zero +required runtime dependencies. zombie-killer-tray is Windows-only (Win32 +process APIs). This integration therefore stays entirely optional and +Windows-gated: nothing here is imported or run unless a caller explicitly +asks for it, and `launch_zombie_killer_watch()` refuses cleanly on any +non-Windows platform instead of attempting anything. + +Same pattern as `dev-bricks/CareCenter-for-Codex`'s existing +`safe_start_integration.py`/`zombie_killer_integration.py`: a git-pinned pip +dependency, launched as its own subprocess via +`python -m zombie_killer_tray watch ...`, coordinated through the shared +zombie_events.jsonl audit log -- never an in-process import. +""" + +from __future__ import annotations + +import json +import os +import signal +import subprocess +import sys +from collections.abc import Callable +from dataclasses import asdict, dataclass +from pathlib import Path + +from .cli import codex_home, no_window_kwargs + +# T-20260926-212716751: zombie-killer-tray#4 (src/-Paketierung) merged as 039b4f2. +ZOMBIE_KILLER_PACKAGE_SPEC = ( + "zombie-killer-tray @ " + "git+https://github.com/dev-bricks/zombie-killer-tray.git" + "@039b4f2c7acd69063b39d6168c757b0b2fca2438" +) +ZOMBIE_KILLER_SOURCE_ENV = "SAFE_START_ZOMBIE_KILLER_SOURCE" +DEFAULT_WATCH_INTERVAL_SECONDS = 600 +DEFAULT_MIN_AGE_SECONDS = 1800 + + +@dataclass(slots=True) +class ZombieKillerInstallResult: + status: str + target: str + command: list[str] + message: str + stdout: str = "" + stderr: str = "" + + def to_dict(self) -> dict[str, object]: + return asdict(self) + + def to_text(self) -> str: + lines = [ + f"Status: {self.status}", + f"Target: {self.target}", + "Command: " + " ".join(self.command), + self.message, + ] + if self.stdout.strip(): + lines.append("Output:") + lines.append(self.stdout.strip()) + if self.stderr.strip(): + lines.append("Error output:") + lines.append(self.stderr.strip()) + return "\n".join(lines) + + +@dataclass(slots=True) +class ZombieKillerLaunchResult: + status: str + command: list[str] + message: str + state_dir: str + pid: int | None = None + + def to_dict(self) -> dict[str, object]: + return asdict(self) + + def to_text(self) -> str: + lines = [f"Status: {self.status}", "Command: " + " ".join(self.command), self.message, + f"State dir: {self.state_dir}"] + if self.pid is not None: + lines.append(f"PID: {self.pid}") + return "\n".join(lines) + + +@dataclass(slots=True) +class ZombieKillerStopResult: + status: str + message: str + pid: int | None = None + + def to_dict(self) -> dict[str, object]: + return asdict(self) + + def to_text(self) -> str: + lines = [f"Status: {self.status}", self.message] + if self.pid is not None: + lines.append(f"PID: {self.pid}") + return "\n".join(lines) + + +@dataclass(slots=True) +class ZombieKillerStatus: + available: bool + supported_platform: bool + state_dir: str + last_cycle_at: float | None + last_cycle_count: int | None + notes: list[str] + + def to_dict(self) -> dict[str, object]: + return asdict(self) + + def to_text(self) -> str: + availability = "installed" if self.available else "not installed" + lines = [f"zombie-killer-tray: {availability}", f"State dir: {self.state_dir}"] + if self.last_cycle_at is not None: + lines.append(f"Last cycle: {self.last_cycle_at} (reaped: {self.last_cycle_count})") + if self.notes: + lines.append("Notes:") + lines.extend(f"- {note}" for note in self.notes) + return "\n".join(lines) + + +def _is_windows() -> bool: + """Testable seam for the platform gate -- monkeypatching `os.name` itself + would break pathlib's own Windows/Posix class dispatch mid-test.""" + return os.name == "nt" + + +def zombie_killer_state_dir() -> Path: + """zombie-killer-tray's own working directory, under CODEX_HOME. + + zombie-killer-tray writes its runtime state (`zombie_events.jsonl`, + `zombie_worker_errors.log`) to its current working directory, not to a + hardcoded path -- this is passed as `cwd=` to the watch subprocess + (see `launch_zombie_killer_watch`). + """ + path = codex_home() / "zombie-killer-tray" + path.mkdir(parents=True, exist_ok=True) + return path + + +def _zombie_killer_importable() -> bool: + try: + __import__("zombie_killer_tray.killer") + except Exception: + return False + return True + + +def _local_zombie_killer_source() -> Path | None: + env_path = os.environ.get(ZOMBIE_KILLER_SOURCE_ENV) + if env_path: + candidate = Path(env_path).expanduser() + if (candidate / "pyproject.toml").exists(): + return candidate + + project_root = Path(__file__).resolve().parents[2] + sibling = project_root.parent / "REL-PUB_zombie-killer-tray" + if (sibling / "pyproject.toml").exists(): + return sibling + return None + + +def zombie_killer_install_target() -> str: + """Prefer a local sibling checkout, otherwise the commit-pinned GitHub source.""" + local_source = _local_zombie_killer_source() + if local_source is not None: + return str(local_source) + return ZOMBIE_KILLER_PACKAGE_SPEC + + +def _pip_command_candidates() -> list[list[str]]: + candidates: list[list[str]] = [] + if not getattr(sys, "frozen", False): + candidates.append([sys.executable, "-m", "pip"]) + candidates.extend((["py", "-3", "-m", "pip"], ["python3", "-m", "pip"], ["python", "-m", "pip"])) + + unique: list[list[str]] = [] + seen: set[tuple[str, ...]] = set() + for candidate in candidates: + key = tuple(candidate) + if key not in seen: + unique.append(candidate) + seen.add(key) + return unique + + +def _resolve_watch_python_executable( + *, runner: Callable[[list[str]], subprocess.CompletedProcess[str]] | None = None +) -> str | None: + """Resolve a REAL python.exe path -- never `py.exe`, the Python Launcher. + + Review finding (T-20260926-212716751): Windows has no exec(), so + launching the watcher via `["py", "-3", ...]` spawns py.exe as a WRAPPER + process that itself spawns the real interpreter as ITS OWN child. The + PID `subprocess.Popen` hands back is the launcher's, not the worker's -- + in a frozen build (`sys.executable` is the packaged host app, unusable + as an interpreter, so `py -3` was the only remaining candidate), + `stop_zombie_killer_watch` was signalling the launcher while the actual + watcher kept running, unreachable, as an orphan. + + Not frozen: `sys.executable` already IS a real interpreter, used + directly. Frozen: resolve the real path once via `-c "import sys; + print(sys.executable)"` through each launcher candidate, so the actual + watch subprocess is spawned directly against that resolved path. + """ + if not getattr(sys, "frozen", False): + return sys.executable + run = runner or _run_install_command + for candidate in (["py", "-3"], ["python3"], ["python"]): + try: + completed = run([*candidate, "-c", "import sys; print(sys.executable)"]) + except OSError: + continue + path = completed.stdout.strip() if completed.stdout else "" + if completed.returncode == 0 and path and Path(path).is_file(): + return path + return None + + +def _run_install_command(command: list[str]) -> subprocess.CompletedProcess[str]: + return subprocess.run( + command, + check=False, + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + ) + + +def install_zombie_killer_package( + *, + target: str | None = None, + runner: Callable[[list[str]], subprocess.CompletedProcess[str]] | None = None, +) -> ZombieKillerInstallResult: + """Install or upgrade zombie-killer-tray via pip -- an explicit user action. + + Callable on any platform (pip will simply fail on a non-Windows box, + since zombie-killer-tray itself requires Win32); `launch_zombie_killer_watch` + is where the platform gate that actually matters lives. + """ + chosen_target = target or zombie_killer_install_target() + run = runner or _run_install_command + attempts: list[ZombieKillerInstallResult] = [] + for pip_command in _pip_command_candidates(): + command = [*pip_command, "install", "--upgrade", chosen_target] + try: + completed = run(command) + except OSError as exc: + attempts.append( + ZombieKillerInstallResult( + status="failed", target=chosen_target, command=command, message=str(exc) + ) + ) + continue + status = "ok" if completed.returncode == 0 else "failed" + result = ZombieKillerInstallResult( + status=status, + target=chosen_target, + command=command, + message=( + "zombie-killer-tray was installed or upgraded." + if status == "ok" + else f"pip exited with code {completed.returncode}." + ), + stdout=completed.stdout or "", + stderr=completed.stderr or "", + ) + if status == "ok": + return result + attempts.append(result) + + if attempts: + last = attempts[-1] + return ZombieKillerInstallResult( + status="failed", + target=chosen_target, + command=last.command, + message="zombie-killer-tray could not be installed.", + stdout=last.stdout, + stderr=last.stderr or last.message, + ) + return ZombieKillerInstallResult( + status="failed", target=chosen_target, command=[], message="No Python/pip command found." + ) + + +def _zombie_killer_env() -> dict[str, str]: + env = os.environ.copy() + local_source = _local_zombie_killer_source() + if local_source is not None: + src = str(local_source / "src") + old_pythonpath = env.get("PYTHONPATH") + env["PYTHONPATH"] = src if not old_pythonpath else src + os.pathsep + old_pythonpath + return env + + +def _watch_pid_file(state_dir: Path) -> Path: + return state_dir / "watch.pid" + + +def _write_watch_pid_file(state_dir: Path, pid: int, create_time: float | None) -> None: + payload = {"pid": pid, "create_time": create_time} + _watch_pid_file(state_dir).write_text(json.dumps(payload), encoding="utf-8") + + +def _read_watch_pid_file(state_dir: Path) -> tuple[int, float | None] | None: + pid_file = _watch_pid_file(state_dir) + if not pid_file.exists(): + return None + try: + payload = json.loads(pid_file.read_text(encoding="utf-8")) + raw_create_time = payload.get("create_time") + return ( + int(payload["pid"]), + float(raw_create_time) if raw_create_time is not None else None, + ) + except (OSError, ValueError, KeyError, TypeError, json.JSONDecodeError): + return None + + +def _process_create_time(pid: int) -> float | None: + try: + import psutil + except ImportError: + return None + try: + return psutil.Process(pid).create_time() + except psutil.Error: + return None + + +def _verify_watch_process(pid: int, expected_create_time: float | None) -> bool | None: + """True: `pid` is verifiably the same zombie-killer-tray watcher incarnation + that was recorded. False: gone, or the PID was reused by something else. + None: cannot verify at all (psutil unavailable or inaccessible) -- + callers MUST treat this as "do not touch" (review finding: `stop` used + to fail-open on None, so a stale, verification-less PID file could + terminate an unrelated process that happened to reuse the PID).""" + try: + import psutil + except ImportError: + return None + try: + proc = psutil.Process(pid) + actual_create_time = proc.create_time() + cmdline = " ".join(proc.cmdline()) + except psutil.Error: + return False + if expected_create_time is not None and abs(actual_create_time - expected_create_time) > 2.0: + return False # a different incarnation -- the pid was reused + return "zombie_killer_tray" in cmdline + + +def launch_zombie_killer_watch( + *, + interval_seconds: int = DEFAULT_WATCH_INTERVAL_SECONDS, + min_age_seconds: int = DEFAULT_MIN_AGE_SECONDS, + apply: bool = True, + popen: Callable[..., subprocess.Popen[str]] | None = None, +) -> ZombieKillerLaunchResult: + """Start `python -m zombie_killer_tray watch` as its own, long-lived + subprocess. + + Refuses cleanly on any non-Windows platform (zombie-killer-tray is + Win32-only) rather than attempting a doomed subprocess spawn. + + `apply=False` starts in preview mode (no `--yes`): the watcher logs + candidates but never terminates a real process. Defaults to `True` for + production use; a caller that only wants to exercise the lifecycle + (does the watcher survive, does stop work) MUST pass `apply=False` -- + otherwise the real subprocess can actually reap real, qualifying + orphans on whatever machine runs it (review finding). + + NO `--parent-pid`: zombie-killer-tray kills the watch process as soon + as whatever PID was passed via `--parent-pid` exits. A one-shot CLI + call like `zombie-killer-watch` exits right after this function + returns, so passing its own (thus already-doomed) PID there killed the + watcher within ~1s of starting it. The watcher's lifecycle is instead + tracked via its own PID file, independent of whoever launched it (see + `stop_zombie_killer_watch`). + + Refuses a second start while the existing PID file describes a still- + running (or unverifiable) watcher -- otherwise a second call overwrites + `watch.pid` and the first watcher becomes unreachable (review finding). + """ + state_dir = zombie_killer_state_dir() + if not _is_windows(): + return ZombieKillerLaunchResult( + status="unsupported-platform", + command=[], + message="zombie-killer-tray is Windows-only; skipped on this platform.", + state_dir=str(state_dir), + ) + + existing = _read_watch_pid_file(state_dir) + if existing is not None: + existing_pid, existing_create_time = existing + verified = _verify_watch_process(existing_pid, existing_create_time) + if verified is not False: + return ZombieKillerLaunchResult( + status="already-running" if verified else "verification-unavailable", + command=[], + message=( + f"A watcher is already running (PID {existing_pid}); " + "use zombie-killer-stop to stop it." + if verified + else "Existing PID file cannot be verified (psutil missing?); " + "check before starting another one." + ), + state_dir=str(state_dir), + pid=existing_pid, + ) + _watch_pid_file(state_dir).unlink(missing_ok=True) # stale/dead -- safe to clear + + python_executable = _resolve_watch_python_executable() + if python_executable is None: + return ZombieKillerLaunchResult( + status="failed", + command=[], + message="No real Python interpreter found (the py.exe launcher is deliberately " + "never used directly as the watch process, see _resolve_watch_python_executable).", + state_dir=str(state_dir), + ) + + args = [ + "watch", + *(["--yes"] if apply else []), + "--interval", str(interval_seconds), + "--min-age", str(min_age_seconds), + ] + command = [python_executable, "-m", "zombie_killer_tray", *args] + env = _zombie_killer_env() + run = popen or subprocess.Popen + try: + process = run(command, cwd=str(state_dir), env=env, close_fds=True, **no_window_kwargs()) + except OSError as exc: + return ZombieKillerLaunchResult( + status="failed", command=command, message=str(exc), state_dir=str(state_dir) + ) + + pid = getattr(process, "pid", None) + pid_int = int(pid) if isinstance(pid, int) else None + if pid_int is None: + return ZombieKillerLaunchResult( + status="failed", + command=command, + message="Subprocess returned no PID.", + state_dir=str(state_dir), + ) + _write_watch_pid_file(state_dir, pid_int, _process_create_time(pid_int)) + return ZombieKillerLaunchResult( + status="ok", + command=command, + message="zombie-killer-tray was started as its own, long-lived watch subprocess.", + state_dir=str(state_dir), + pid=pid_int, + ) + + +def stop_zombie_killer_watch() -> ZombieKillerStopResult: + """Stop the watch subprocess started by `launch_zombie_killer_watch`, + identified via its PID file rather than any parent/child relationship.""" + state_dir = zombie_killer_state_dir() + existing = _read_watch_pid_file(state_dir) + if existing is None: + return ZombieKillerStopResult(status="not-running", message="No PID file found.") + pid, create_time = existing + + verified = _verify_watch_process(pid, create_time) + if verified is None: + # FAIL-CLOSED (review finding): cannot verify this PID is really our + # watcher (psutil missing/inaccessible) -- refuse to touch it rather + # than blindly signalling a possibly-unrelated, reused PID. + return ZombieKillerStopResult( + status="verification-unavailable", + message="Cannot verify PID (psutil missing?); process was NOT stopped.", + pid=pid, + ) + if verified is False: + _watch_pid_file(state_dir).unlink(missing_ok=True) + return ZombieKillerStopResult( + status="not-found", message="PID no longer belongs to zombie-killer-tray.", pid=pid + ) + + try: + os.kill(pid, signal.SIGTERM) + except OSError: + _watch_pid_file(state_dir).unlink(missing_ok=True) + return ZombieKillerStopResult(status="already-stopped", message="Process was not running.", pid=pid) + + _watch_pid_file(state_dir).unlink(missing_ok=True) + return ZombieKillerStopResult(status="ok", message="zombie-killer-tray was stopped.", pid=pid) + + +def _last_cycle_event(state_dir: Path) -> dict[str, object] | None: + events_path = state_dir / "zombie_events.jsonl" + if not events_path.exists(): + return None + last: dict[str, object] | None = None + try: + with events_path.open("r", encoding="utf-8", errors="replace") as handle: + for line in handle: + line = line.strip() + if not line: + continue + try: + record = json.loads(line) + except json.JSONDecodeError: + continue + if isinstance(record, dict) and "cycle_at" in record: + last = record + except OSError: + return None + return last + + +def build_zombie_killer_status() -> ZombieKillerStatus: + state_dir = zombie_killer_state_dir() + notes: list[str] = [] + supported = _is_windows() + if not supported: + notes.append("zombie-killer-tray is Windows-only; not applicable on this platform.") + available = supported and _zombie_killer_importable() + if supported and not available: + notes.append("zombie-killer-tray package not importable; showing prior audit log if any.") + + last_cycle = _last_cycle_event(state_dir) + return ZombieKillerStatus( + available=available, + supported_platform=supported, + state_dir=str(state_dir), + last_cycle_at=float(last_cycle["cycle_at"]) if last_cycle else None, + last_cycle_count=int(last_cycle["count"]) if last_cycle and "count" in last_cycle else None, + notes=notes, + ) diff --git a/tests/test_third_party_licenses.py b/tests/test_third_party_licenses.py index 19f4497..c9bff9a 100644 --- a/tests/test_third_party_licenses.py +++ b/tests/test_third_party_licenses.py @@ -37,6 +37,8 @@ def test_third_party_license_inventory_covers_direct_dependencies() -> None: "pystray", "pytest", "ruff", + "zombie-killer-tray", + "psutil", } for package_name in _declared_direct_dependencies(): assert package_name in text diff --git a/tests/test_zombie_killer_integration.py b/tests/test_zombie_killer_integration.py new file mode 100644 index 0000000..9e84824 --- /dev/null +++ b/tests/test_zombie_killer_integration.py @@ -0,0 +1,399 @@ +from __future__ import annotations + +import contextlib +import json +import os +import signal +import subprocess +import sys +import time +from pathlib import Path +from types import SimpleNamespace + +import pytest + +from safe_start_for_codex.zombie_killer_integration import ( + ZOMBIE_KILLER_PACKAGE_SPEC, + ZOMBIE_KILLER_SOURCE_ENV, + build_zombie_killer_status, + install_zombie_killer_package, + launch_zombie_killer_watch, + stop_zombie_killer_watch, + zombie_killer_install_target, + zombie_killer_state_dir, +) + + +def test_state_dir_is_under_codex_home(tmp_path: Path, monkeypatch) -> None: + monkeypatch.setenv("CODEX_HOME", str(tmp_path / ".codex")) + state_dir = zombie_killer_state_dir() + assert state_dir == tmp_path / ".codex" / "zombie-killer-tray" + assert state_dir.is_dir(), "must create the directory" + + +def test_install_target_falls_back_to_pinned_github_spec_without_local_source( + monkeypatch, +) -> None: + monkeypatch.delenv(ZOMBIE_KILLER_SOURCE_ENV, raising=False) + monkeypatch.setattr( + "safe_start_for_codex.zombie_killer_integration._local_zombie_killer_source", + lambda: None, + ) + assert zombie_killer_install_target() == ZOMBIE_KILLER_PACKAGE_SPEC + + +def test_install_target_prefers_local_source_env_override(tmp_path: Path, monkeypatch) -> None: + local = tmp_path / "local-zkt" + local.mkdir() + (local / "pyproject.toml").write_text("[project]\nname='x'\n", encoding="utf-8") + monkeypatch.setenv(ZOMBIE_KILLER_SOURCE_ENV, str(local)) + assert zombie_killer_install_target() == str(local) + + +def test_install_zombie_killer_package_reports_ok_on_zero_exit() -> None: + calls: list[list[str]] = [] + + def fake_runner(command: list[str]) -> subprocess.CompletedProcess[str]: + calls.append(command) + return subprocess.CompletedProcess(command, returncode=0, stdout="installed", stderr="") + + result = install_zombie_killer_package(target="some-target", runner=fake_runner) + + assert result.status == "ok" + assert calls[0][-3:] == ["install", "--upgrade", "some-target"] + + +def test_install_zombie_killer_package_reports_failed_on_nonzero_exit() -> None: + def fake_runner(command: list[str]) -> subprocess.CompletedProcess[str]: + return subprocess.CompletedProcess(command, returncode=1, stdout="", stderr="boom") + + result = install_zombie_killer_package(target="some-target", runner=fake_runner) + + assert result.status == "failed" + assert "boom" in result.stderr + + +def test_launch_refuses_cleanly_on_non_windows(tmp_path: Path, monkeypatch) -> None: + monkeypatch.setenv("CODEX_HOME", str(tmp_path / ".codex")) + monkeypatch.setattr( + "safe_start_for_codex.zombie_killer_integration._is_windows", lambda: False + ) + + called = False + + def fake_popen(command, **kwargs): + nonlocal called + called = True + return SimpleNamespace(pid=1) + + result = launch_zombie_killer_watch(popen=fake_popen) + + assert result.status == "unsupported-platform" + assert called is False, "must never attempt to spawn on a non-Windows platform" + + +def test_launch_invokes_module_with_expected_args_on_windows(tmp_path: Path, monkeypatch) -> None: + monkeypatch.setenv("CODEX_HOME", str(tmp_path / ".codex")) + captured: dict[str, object] = {} + + def fake_popen(command, **kwargs): + captured["command"] = command + captured["cwd"] = kwargs.get("cwd") + return SimpleNamespace(pid=4242) + + result = launch_zombie_killer_watch(interval_seconds=42, min_age_seconds=99, popen=fake_popen) + + assert result.status == "ok" + assert result.pid == 4242 + command = captured["command"] + assert "-m" in command + assert "zombie_killer_tray" in command + assert "watch" in command + assert "--interval" in command and "42" in command + assert "--min-age" in command and "99" in command + assert "--yes" in command + assert "--parent-pid" not in command, ( + "must NOT tie the watcher's lifetime to this (necessarily short-lived) " + "caller's own PID -- it would die within ~1s of being spawned (review finding)" + ) + pid_payload = json.loads((zombie_killer_state_dir() / "watch.pid").read_text(encoding="utf-8")) + assert pid_payload["pid"] == 4242 + assert captured["cwd"] == str(tmp_path / ".codex" / "zombie-killer-tray") + + +def test_launch_uses_preview_mode_without_apply(tmp_path: Path, monkeypatch) -> None: + """Review finding: a caller that only wants to exercise the watcher's + lifecycle, not its actual reap behaviour, must be able to omit --yes -- + otherwise a real watch subprocess can terminate real, qualifying orphans + on whatever machine runs it.""" + monkeypatch.setenv("CODEX_HOME", str(tmp_path / ".codex")) + + def fake_popen(command, **kwargs): + return SimpleNamespace(pid=1) + + result = launch_zombie_killer_watch(apply=False, popen=fake_popen) + assert result.status == "ok" + assert "--yes" not in result.command + + +def test_launch_refuses_a_second_start_while_the_first_watcher_is_verified_alive( + tmp_path: Path, monkeypatch +) -> None: + """Review finding: a second `watch` used to silently overwrite watch.pid, + orphaning the first watcher (still running, now unreachable via stop).""" + monkeypatch.setenv("CODEX_HOME", str(tmp_path / ".codex")) + state_dir = zombie_killer_state_dir() + (state_dir / "watch.pid").write_text( + json.dumps({"pid": 424242, "create_time": 123.0}), encoding="utf-8" + ) + monkeypatch.setattr( + "safe_start_for_codex.zombie_killer_integration._verify_watch_process", + lambda pid, create_time: True, + ) + called = False + + def fake_popen(command, **kwargs): + nonlocal called + called = True + return SimpleNamespace(pid=1) + + result = launch_zombie_killer_watch(popen=fake_popen) + + assert result.status == "already-running" + assert result.pid == 424242 + assert called is False, "must never spawn a second watcher over a verified-alive one" + assert json.loads((state_dir / "watch.pid").read_text(encoding="utf-8"))["pid"] == 424242 + + +def test_launch_refuses_a_second_start_when_verification_is_unavailable( + tmp_path: Path, monkeypatch +) -> None: + """Fail-closed applies to the double-start guard too.""" + monkeypatch.setenv("CODEX_HOME", str(tmp_path / ".codex")) + state_dir = zombie_killer_state_dir() + (state_dir / "watch.pid").write_text( + json.dumps({"pid": 424242, "create_time": 123.0}), encoding="utf-8" + ) + monkeypatch.setattr( + "safe_start_for_codex.zombie_killer_integration._verify_watch_process", + lambda pid, create_time: None, + ) + called = False + + def fake_popen(command, **kwargs): + nonlocal called + called = True + return SimpleNamespace(pid=1) + + result = launch_zombie_killer_watch(popen=fake_popen) + + assert result.status == "verification-unavailable" + assert called is False + + +def test_launch_proceeds_when_the_existing_pid_file_is_stale(tmp_path: Path, monkeypatch) -> None: + monkeypatch.setenv("CODEX_HOME", str(tmp_path / ".codex")) + state_dir = zombie_killer_state_dir() + (state_dir / "watch.pid").write_text( + json.dumps({"pid": 424242, "create_time": 123.0}), encoding="utf-8" + ) + monkeypatch.setattr( + "safe_start_for_codex.zombie_killer_integration._verify_watch_process", + lambda pid, create_time: False, + ) + + def fake_popen(command, **kwargs): + return SimpleNamespace(pid=99999) + + result = launch_zombie_killer_watch(popen=fake_popen) + + assert result.status == "ok" + assert result.pid == 99999 + assert json.loads((state_dir / "watch.pid").read_text(encoding="utf-8"))["pid"] == 99999 + + +def test_resolve_python_executable_uses_sys_executable_when_not_frozen() -> None: + from safe_start_for_codex.zombie_killer_integration import _resolve_watch_python_executable + + assert _resolve_watch_python_executable() == sys.executable + + +def test_resolve_python_executable_resolves_through_the_launcher_when_frozen(monkeypatch) -> None: + """Review finding: in a frozen build, `sys.executable` is the packaged + host app, not an interpreter, and launching via the bare `py -3` + launcher would hand back the LAUNCHER's pid, not the worker's.""" + from safe_start_for_codex.zombie_killer_integration import _resolve_watch_python_executable + + monkeypatch.setattr(sys, "frozen", True, raising=False) + real_path = sys.executable + + def fake_runner(command: list[str]) -> subprocess.CompletedProcess[str]: + assert command[:2] == ["py", "-3"] + return subprocess.CompletedProcess(command, returncode=0, stdout=real_path + "\n", stderr="") + + resolved = _resolve_watch_python_executable(runner=fake_runner) + assert resolved == real_path + + +def test_launch_reports_failure_when_no_python_found(tmp_path: Path, monkeypatch) -> None: + monkeypatch.setenv("CODEX_HOME", str(tmp_path / ".codex")) + + def failing_popen(command, **kwargs): + raise OSError("no interpreter") + + result = launch_zombie_killer_watch(popen=failing_popen) + + assert result.status == "failed" + assert "no interpreter" in result.message + + +def test_build_status_reads_last_cycle_from_events_log(tmp_path: Path, monkeypatch) -> None: + monkeypatch.setenv("CODEX_HOME", str(tmp_path / ".codex")) + state_dir = zombie_killer_state_dir() + events = state_dir / "zombie_events.jsonl" + with events.open("w", encoding="utf-8") as handle: + handle.write(json.dumps({"cycle_at": 111.0, "apply": False, "count": 0}) + "\n") + handle.write(json.dumps({"cycle_at": 222.0, "apply": True, "count": 3}) + "\n") + handle.write(json.dumps({"event": "broker-superseded-idle", "root_pid": 5}) + "\n") + + status = build_zombie_killer_status() + + assert status.last_cycle_at == 222.0 + assert status.last_cycle_count == 3 + assert status.state_dir == str(state_dir) + + +def test_build_status_handles_missing_log(tmp_path: Path, monkeypatch) -> None: + monkeypatch.setenv("CODEX_HOME", str(tmp_path / ".codex")) + + status = build_zombie_killer_status() + + assert status.last_cycle_at is None + assert status.last_cycle_count is None + + +def test_build_status_marks_non_windows_as_unsupported(tmp_path: Path, monkeypatch) -> None: + monkeypatch.setenv("CODEX_HOME", str(tmp_path / ".codex")) + monkeypatch.setattr( + "safe_start_for_codex.zombie_killer_integration._is_windows", lambda: False + ) + + status = build_zombie_killer_status() + + assert status.supported_platform is False + assert status.available is False + assert any("Windows-only" in note for note in status.notes) + + +def test_stop_reports_not_running_without_a_pid_file(tmp_path: Path, monkeypatch) -> None: + monkeypatch.setenv("CODEX_HOME", str(tmp_path / ".codex")) + result = stop_zombie_killer_watch() + assert result.status == "not-running" + + +def test_stop_reports_not_found_for_a_stale_pid_reused_by_something_else( + tmp_path: Path, monkeypatch +) -> None: + pytest.importorskip("psutil") + monkeypatch.setenv("CODEX_HOME", str(tmp_path / ".codex")) + state_dir = zombie_killer_state_dir() + # PID of the current test process itself -- definitely alive, definitely + # NOT a zombie-killer-tray process (wrong cmdline AND, deliberately, a + # create_time far from its real one), so this exercises the reuse check + # refusing to kill an unrelated process that happens to reuse the pid. + (state_dir / "watch.pid").write_text( + json.dumps({"pid": os.getpid(), "create_time": 1.0}), encoding="utf-8" + ) + + result = stop_zombie_killer_watch() + + assert result.status == "not-found" + assert not (state_dir / "watch.pid").exists(), "stale/wrong pid file must be cleaned up" + + +def test_stop_fails_closed_when_verification_is_unavailable(tmp_path: Path, monkeypatch) -> None: + """Review finding (blocking): stop used to fail-OPEN when verification + returned None (psutil unavailable) -- it killed the PID anyway. It must + instead refuse and leave the process untouched.""" + monkeypatch.setenv("CODEX_HOME", str(tmp_path / ".codex")) + state_dir = zombie_killer_state_dir() + (state_dir / "watch.pid").write_text( + json.dumps({"pid": 999999999, "create_time": 123.0}), encoding="utf-8" + ) + killed: list[int] = [] + monkeypatch.setattr( + "safe_start_for_codex.zombie_killer_integration._verify_watch_process", + lambda pid, create_time: None, + ) + monkeypatch.setattr(os, "kill", lambda pid, sig: killed.append(pid)) + + result = stop_zombie_killer_watch() + + assert result.status == "verification-unavailable" + assert killed == [], "must NOT signal the pid when verification is unavailable (fail-closed)" + assert (state_dir / "watch.pid").exists(), "an unresolved pid file is left in place, not deleted" + + +def test_stop_reports_already_stopped_for_a_verified_but_dead_pid( + tmp_path: Path, monkeypatch +) -> None: + monkeypatch.setenv("CODEX_HOME", str(tmp_path / ".codex")) + state_dir = zombie_killer_state_dir() + (state_dir / "watch.pid").write_text( + json.dumps({"pid": 999999999, "create_time": 123.0}), encoding="utf-8" + ) + monkeypatch.setattr( + "safe_start_for_codex.zombie_killer_integration._verify_watch_process", + lambda pid, create_time: True, + ) + monkeypatch.setattr(os, "kill", lambda pid, sig: (_ for _ in ()).throw(OSError("gone"))) + + result = stop_zombie_killer_watch() + + assert result.status == "already-stopped" + assert not (state_dir / "watch.pid").exists() + + +@pytest.mark.skipif(os.name != "nt", reason="zombie-killer-tray is Windows-only") +@pytest.mark.timeout(30) +def test_real_watch_subprocess_outlives_its_launcher_and_stop_terminates_it( + tmp_path: Path, monkeypatch +) -> None: + """No mocked Popen -- a genuinely real subprocess, spawned exactly the way + production code does it. Direct regression test for the review finding: + the old design passed --parent-pid , and + zombie-killer-tray's watch_parent thread kills the watcher within ~1s of + whatever PID it was given exiting. + """ + psutil = pytest.importorskip("psutil") + pytest.importorskip("zombie_killer_tray") + monkeypatch.setenv("CODEX_HOME", str(tmp_path / ".codex")) + + # apply=False (preview/no --yes): review finding -- with --yes, this real + # subprocess would actually reap real, qualifying orphan processes on + # whatever machine runs this test (including CI). + result = launch_zombie_killer_watch(interval_seconds=3, min_age_seconds=30, apply=False) + assert result.status == "ok", result.message + assert "--yes" not in result.command + pid = result.pid + assert pid is not None + + try: + time.sleep(2.0) + assert psutil.pid_exists(pid), ( + "the watcher must still be running 2s after launch returned -- " + "with the old --parent-pid design it would already be dead" + ) + + stop_result = stop_zombie_killer_watch() + assert stop_result.status == "ok" + assert stop_result.pid == pid + + deadline = time.monotonic() + 10 + while time.monotonic() < deadline and psutil.pid_exists(pid): + time.sleep(0.2) + assert not psutil.pid_exists(pid), "the watcher must actually exit after being stopped" + finally: + if psutil.pid_exists(pid): + with contextlib.suppress(OSError): + os.kill(pid, signal.SIGTERM)