diff --git a/scripts/ci/check_os_marker_fakes.py b/scripts/ci/check_os_marker_fakes.py new file mode 100644 index 0000000000..18f90690b6 --- /dev/null +++ b/scripts/ci/check_os_marker_fakes.py @@ -0,0 +1,135 @@ +#!/usr/bin/env python3 +"""Fail when a test file fakes macOS without carrying ``@pytest.mark.macos_only``. + +The OS lanes are marker-driven: ``.github/workflows/tests-os.yml`` selects the +files the macOS job imports via ``scripts/ci/list_os_marked_tests.py macos_only`` +and then runs ``-m macos_only``. A file whose tests only pass because they make +the interpreter believe it is on macOS (``is_macos`` patched to ``True``, +``sys.platform`` set to ``"darwin"``) but that carries no marker is invisible to +that lane: it is green on Linux over a faked branch and never imported on the +host it exists for (#111866). Root ``AGENTS.md`` § "Don't fake the host OS" is +the rule; this check makes a violation a red job instead of a review catch. + +Flags, per ``tests/**/test_*.py`` without a whole-word ``macos_only``: + + monkeypatch.setattr(mod, "is_macos", lambda: True) / patch(..., return_value=True) + monkeypatch.setattr(sys, "platform", "darwin") / patch("sys.platform", "darwin") + platform.system patched to return "Darwin" + +Opt out of one line with ``# os-marker: ok — `` on that line (a pure +function taking the platform as data is host-independent and stays unmarked). +``_BASELINE`` lists the files that already faked macOS when this check landed; +they are a burn-down list, not a policy — split the macOS arm out, mark it, +and drop the entry (a stale entry fails the check). + +Run: python scripts/ci/check_os_marker_fakes.py [tests_root] +""" + +from __future__ import annotations + +import os +import re +import sys +from pathlib import Path + +MARKER = "macos_only" +OPT_OUT = "os-marker: ok" + +_TRUE = r"(?:lambda[^:]*:\s*True|return_value\s*=\s*True|,\s*True\b)" +_FAKES = ( + re.compile(rf"\bis_macos\b.*{_TRUE}"), + re.compile(r"\bis_macos\.return_value\s*=\s*True\b"), + # setattr(sys, "platform", "darwin") / patch("sys.platform", "darwin") / x.platform = "darwin"; + # a host-honest READ (`if sys.platform == "darwin":`) is not a fake and does not match. + re.compile(r"""\bplatform["']?\s*,\s*["']darwin["']"""), + re.compile(r"""\.platform\s*=\s*["']darwin["']"""), + re.compile(r"""\bplatform\.system\b.*(?:lambda[^:]*:|return_value\s*=)\s*["']Darwin["']"""), +) + +# Files that faked macOS before this check existed (#111866). Burn down, never extend. +_BASELINE = frozenset( + { + "tests/hermes_cli/test_doctor.py", + "tests/hermes_cli/test_gateway.py", + "tests/hermes_cli/test_gateway_proc_fallback.py", + "tests/hermes_cli/test_linux_sandbox_fixup.py", + "tests/hermes_cli/test_macos_fda_guidance.py", + "tests/hermes_cli/test_orphan_desktop_serve_reap.py", + "tests/hermes_cli/test_update_launchd_restart_verification.py", + "tests/hermes_cli/test_update_launchd_unloaded_gateway.py", + "tests/hermes_cli/test_urllib_security.py", + "tests/hermes_state/test_state_synchronous_pragma.py", + "tests/test_hermes_constants.py", + "tests/tools/test_macos_protected_search.py", + "tests/tools/test_skills_tool.py", + } +) + + +def _code_lines(text: str) -> list[tuple[int, str, str]]: + """Yield ``(lineno, code, raw)`` with the ``#`` comment stripped from *code*. + + A ``#`` inside a string literal is rare in these patterns and only ever + hides a hit (never invents one), so a plain split is the honest trade + against a full tokenizer. + """ + out = [] + for i, raw in enumerate(text.splitlines(), 1): + out.append((i, raw.split("#", 1)[0], raw)) + return out + + +def find_unmarked_fakes(root: Path, repo_root: Path) -> dict[str, list[tuple[int, str]]]: + """Map repo-relative test path -> ``[(lineno, line)]`` of un-opted-out macOS fakes.""" + marker_pat = re.compile(rf"\b{MARKER}\b") + hits: dict[str, list[tuple[int, str]]] = {} + for dirpath, _dirnames, filenames in os.walk(root): + for fname in filenames: + if not (fname.startswith("test_") and fname.endswith(".py")): + continue + path = Path(dirpath) / fname + try: + text = path.read_text(encoding="utf-8", errors="replace") + except OSError: + continue + if marker_pat.search(text): + continue + lines = [ + (n, raw.strip()) + for n, code, raw in _code_lines(text) + if OPT_OUT not in raw and any(p.search(code) for p in _FAKES) + ] + if lines: + resolved = path.resolve() + base = repo_root if resolved.is_relative_to(repo_root) else root.resolve() + hits[resolved.relative_to(base).as_posix()] = lines + return hits + + +def main(argv: list[str]) -> int: + repo_root = Path(__file__).resolve().parents[2] + root = Path(argv[1]) if len(argv) > 1 else repo_root / "tests" + if not root.is_dir(): + print(f"error: no such directory: {root}", file=sys.stderr) + return 2 + hits = find_unmarked_fakes(root, repo_root) + new = {rel: lines for rel, lines in hits.items() if rel not in _BASELINE} + stale = sorted(_BASELINE - set(hits)) + for rel, lines in sorted(new.items()): + for n, line in lines: + print(f"{rel}:{n}: fakes macOS without @pytest.mark.{MARKER}: {line}") + if new: + print( + f"\n{len(new)} test file(s) make the interpreter believe it is on macOS but carry no " + f"`{MARKER}` marker, so the macOS lane never imports them (AGENTS.md § Don't fake the " + f"host OS). Split the macOS arm into its own `@pytest.mark.{MARKER}` test that runs the " + f"real branch, or mark `# {OPT_OUT} — ` on a host-independent line.", + file=sys.stderr, + ) + for rel in stale: + print(f"{rel}: listed in _BASELINE but no longer fakes macOS — remove the entry") + return 1 if new or stale else 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv)) diff --git a/tests/ci/test_check_os_marker_fakes.py b/tests/ci/test_check_os_marker_fakes.py new file mode 100644 index 0000000000..1133307323 --- /dev/null +++ b/tests/ci/test_check_os_marker_fakes.py @@ -0,0 +1,39 @@ +"""The unmarked-macOS-fake guard (#111866): flags fakes, ignores honest reads and marked files.""" + +from __future__ import annotations + +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parents[2] / "scripts" / "ci")) + +from check_os_marker_fakes import find_unmarked_fakes # noqa: E402 + + +def _write(root: Path, name: str, body: str) -> None: + (root / name).write_text(body, encoding="utf-8") + + +def test_unmarked_fake_is_flagged_marked_and_opted_out_are_not(tmp_path): + _write(tmp_path, "test_fake.py", "def test_x(monkeypatch):\n" + " monkeypatch.setattr(gw, 'is_macos', lambda: True)\n" + " monkeypatch.setattr(sys, 'platform', 'darwin')\n") + _write(tmp_path, "test_marked.py", "import pytest\npytestmark = pytest.mark.macos_only\n" + "def test_x(monkeypatch):\n monkeypatch.setattr(gw, 'is_macos', lambda: True)\n") + _write(tmp_path, "test_opted.py", "def test_x(monkeypatch):\n" + " patch('m.is_macos', return_value=True) # os-marker: ok — pure data mapping\n") + + hits = find_unmarked_fakes(tmp_path, tmp_path) + + assert set(hits) == {"test_fake.py"} + assert [n for n, _ in hits["test_fake.py"]] == [2, 3] + + +def test_host_honest_platform_read_is_not_a_fake(tmp_path): + _write(tmp_path, "test_read.py", "import sys\n" + "def test_x():\n expected = sys.platform == 'darwin'\n" + " if sys.platform == 'darwin':\n pass\n" + " payload = {'platform': 'darwin'}\n" + " # monkeypatch.setattr(gw, 'is_macos', lambda: True) in a comment\n") + + assert find_unmarked_fakes(tmp_path, tmp_path) == {} diff --git a/tests/hermes_cli/test_update_autostash.py b/tests/hermes_cli/test_update_autostash.py index 5f4c7eb179..6ac9c04e48 100644 --- a/tests/hermes_cli/test_update_autostash.py +++ b/tests/hermes_cli/test_update_autostash.py @@ -57,9 +57,14 @@ def _patch_gateway_discovery(): phase's fresh ``from hermes_cli.gateway import ...`` then loads an UNPATCHED copy of the module — silently discarding every mock here and letting real gateway discovery (and real ``os.kill``) run on the dev box. + + The launchd scope is neutralised too: on a macOS host the restart phase + derives labels from the profile layout, so a default profile alone hands + it ``ai.hermes.gateway`` and the verify step exits 1 (#111866, #110701). """ with patch("hermes_cli.gateway.find_gateway_pids", return_value=[]), \ patch("hermes_cli.gateway.supports_systemd_services", return_value=False), \ + patch("hermes_cli.update_cmd_fleet._restart_macos_launchd_gateways", lambda *a, **k: None), \ patch("hermes_cli.gateway.find_profile_gateway_processes", return_value=[]), \ patch("hermes_cli.update_inventory.collect_runtime_inventory", return_value=None), \ patch("hermes_cli.update_inventory.report_unaccounted_runtimes", return_value=False), \