ouroboros/tests/test_platform_guard.py
ndrew1337 45d77ff785
tests+ci: green & de-flake the pre-push suite (P3/P6/xdist-safety) + parallelize CI ~9x (gate stays serial) (#55)
* test(improvement_backlog): stub semantic-dedup LLM in groom tests (P6)

_seed_many() seeds items via append_backlog_items(), whose C9.2 semantic-redirect
pre-pass calls semantic_dedup.find_semantic_duplicate_id() once per fingerprint-MISS
with candidates — a real light-model NETWORK call. The seeding runs BEFORE
_patch_groom_llm installs its mock, and that mock only covers chat_observed, not the
detector's own client path. With no API key the call retry-storms for minutes before
failing open to None, making test_groom_backlog_rejects_invented_items ~129s alone
(~40% of the whole suite) and non-deterministic.

Add a module-level autouse fixture that stubs find_semantic_duplicate_id to its own
fail-open default (None = no duplicate — exactly what the doomed call eventually
returns for the distinct seeded items), so the module is network-free and
deterministic. No production change; the dedup contract stays covered by
test_semantic_dedup_v6370.

Effect: test_groom_backlog_rejects_invented_items 129.12s -> 0.02s; whole file
~165s -> 2.37s, all 14 tests green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(core): validate regex up front in _code_search so invalid-regex contract holds for both backends (P3)

_code_search ran the ripgrep path first and returned its formatted result before
ever reaching the Python-fallback re.compile guard. ripgrep accepts some malformed
patterns permissively — an unterminated '[' yields "no matches" instead of erroring
— so an invalid regex like "[invalid" silently returned no-match on the rg path while
only the fallback (rg absent/failed) emitted "⚠️ SEARCH_ERROR: invalid regex". This
made test_code_search_invalid_regex fail whenever ripgrep is present (i.e. always, in
CI and the pre-push preflight), so the suite exited non-zero on every candidate diff
regardless of the change under test.

Compile the regex once up front (regex queries only; literal queries need no check)
and return SEARCH_ERROR on re.error before dispatching to either backend, so both the
rg path and the Python fallback share the same invalid-regex contract.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test: guard the conftest repo-root pollution sweep to the xdist controller (xdist-safety)

Under pytest-xdist, pytest_sessionfinish fires on the controller AND every worker against
the SHARED repo root, so the mock-pollution sweep (shutil.rmtree of leaked <MagicMock>
paths + session.exitstatus=1) had workers racing the same rmtree and each independently
failing the run — a non-deterministic, failed-shaped result. Guard the repo-root sweep +
exitstatus mutation behind `if not hasattr(session.config, "workerinput")` so it runs only
on the controller (the single authority); the per-process _PYTEST_DATA_DIR cleanup stays
outside the guard and runs on every process. Serial runs are unaffected (the guard is
always True without -n).

Adds tests/test_conftest_xdist_guard.py (sweep runs on the controller, skipped on a worker).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* ci: parallelize the test suite with pytest-xdist (~9x faster), keep the gate serial

The full-suite CI jobs (quick-test, full-test) now run a PARALLEL pass plus a short SERIAL
pass. Empirically ~270s serial -> ~30s parallel (~9x). The per-commit preflight GATE stays
serial by design (a flaky parallel fail-closed gate manufactures non-deterministic
TESTS_FAILED indistinguishable from a real immune rejection).

- requirements: pytest-xdist + pytest-timeout (a hang-guard).
- pyproject: register the `serial` marker (addopts unchanged).
- ci.yml (quick-test, full-test): two-step. Parallel
  `-m "not serial and <default lane exclusions>" -n auto --dist loadscope
  --max-worker-restart=0 --timeout=300 --timeout-method=thread`, then serial
  `-m "serial and <exclusions>"`. A command-line -m REPLACES the pyproject addopts markexpr,
  so the default lane exclusions are repeated and ANDed with the serial split — the union
  exactly reproduces the old default suite (4594 parallel + 122 serial = 4716, disjoint).
- conftest: a tryfirst pytest_collection_modifyitems hook marks the real-process/port files
  (workspace_executor[+cleanup], process_custody, kill_process_tree_orphans, zombie_prevention,
  worker_crash_retry, process_resource_leaks, restart_reconnect, preflight_runner,
  services_tool_v2) `serial`; plus an autouse fixture isolating workspace_executor._SERVICES/
  _FOREGROUND between tests (a latent ordering bug -n redistribution exposes).

Test-isolation fixes surfaced by running the suite under -n:
- test_task_constraint_tools: 3 bare `sys.modules["...claude_code"] = mock` (no restore ->
  polluted the worker's sys.modules -> later SDK-dependent tests failed) -> monkeypatch.setitem.
- test_workspace_executor: poll until the spawned process's command-sha is readable before
  registering (the PID-reuse safety check compared the sha recorded at registration vs
  recomputed at kill; right after fork+exec the command line is unreadable -> shas diverge ->
  kill silently skipped -> flaky), + 5->15s kill-confirmation deadlines.

docs/DEVELOPMENT.md + docs/CHECKLISTS.md: guidance so future tests are parallel-safe or
marked `serial`. Reviewed by adversarial subagents (ship); verified parallel 8/8 + serial 3/3
green, partition exact + disjoint, gate byte-identical.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* tests+ci: parallel-safety follow-ups (services-global isolation, serial-lane guard, monkeypatch conversions)

Three non-blocking follow-ups from the parallelization review, hardening the
xdist split shipped earlier in this PR:

F1 (tests/conftest.py): extend the autouse _isolate_workspace_executor_globals
fixture to ALSO snapshot/clear/restore ouroboros.tools.services._SERVICES (the
legacy services registry, a separate module-global from workspace_executor's)
under its plain threading.Lock _LOCK. Closes the legacy-services-path
global-leak class generally; raw dict ops only under the lock (no re-entrant
deadlock on the plain Lock), registry-only (never reaps the live Popen
handles), each module lazy-imported under its own guard.

F2 (.github/workflows/ci.yml): add a "Guard non-empty serial marker lane" step
to marker-guards. `pytest --collect-only -m serial` exits 5 on an emptied
_SERIAL_TEST_FILES; set -euo pipefail + tee surfaces it, and a positive anchor
grep on test_workspace_executor.py is the working assertion (the existing
browser-guard's `! grep "no tests collected"` is a dead no-op under -q).

F3 (test_skill_loader / test_iteration2_fixes / test_marketplace_clawhub /
test_devtools_benchmarks): convert remaining bare os.environ / sys.modules
mutations (no save-restore) to auto-reverting monkeypatch.setenv/delenv/setitem,
so the parallel suite has no cross-test env/module leaks. tests/_shared.py's
intentional process-wide SDK mock left untouched.

Verified: parallel lane 4574 pass (-n auto --dist loadscope), serial lane 140
pass, the 4 converted files 159 pass. Two subagent reviews: no regressions.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test: prune 26 redundant/tautological/obsolete tests (immune coverage retained)

Three-pass audit (finder+skeptic -> 31 empirical verifiers with rename+mutation
experiments -> delete-and-run dry-run) of the 275-file / 4067-test suite found 26
tests whose value is fully backstopped by a stronger survivor or is near-zero:

- tautological / zero-production pins (assert hasattr/callable; inline-simulated
  branches; closed-loop regex; TypedDict dict-literal; CPython-only checks);
- proven duplicates with strict-superset twins (test_consolidator, test_commit_gate,
  test_cache_optimization, test_review_intent_split, TestGrepRegexHint, ...);
- registry-registration one-liners, all backstopped by test_smoke::test_tool_set_matches
  (set-equality vs EXPECTED_TOOLS -- mutation-proven to FAIL on a lost registration);
- dead-feature tests (retired skill_migrations module) + obsolete version regressions
  (v636 import now unconditional; README "(N tests)" convention abandoned).

Removes ~47 test functions across 20 files (incl. the whole test_shell_regex_hint.py,
mirrored 1:1 in test_shell_run_shell.py::TestGrepRegexHint, and the whole
TestGoalScopePrecedence class). Also drops 2 now-unused imports + 1 orphaned helper.

KEPT (not redundant): test_smoke::test_git_commit_with_tests_exists -- sole guard of the
post-commit gate seam (a rename experiment proved nothing else catches its loss).

Verified: ruff F clean; collection exit 0 (4703 collected, no emptied class); full
parallel + serial suites green; two subagent reviews confirm exact set + no broken refs.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Andrew <andgri200@gmail.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-26 02:56:34 +03:00

227 lines
8.3 KiB
Python

"""
AST-based guard: platform-specific APIs must live in platform_layer.py only.
Scans all .py files under ouroboros/ and supervisor/ (plus server.py)
for direct use of the following forbidden patterns:
- Top-level imports of platform-specific modules: fcntl, msvcrt, winreg, resource
- Direct attribute access: os.kill, os.killpg, os.setsid, os.getpgid
- Direct attribute access: signal.SIGKILL, signal.SIGTERM
Note: subprocess flags (creationflags, start_new_session) and launcher.py
are NOT covered by this guard — platform_layer.py provides
subprocess_new_group_kwargs() and subprocess_hidden_kwargs() helpers,
and caller code uses them (checked by code review, not by this AST guard).
launcher.py is the immutable outer shell and is intentionally excluded.
Runs on every `make test` on all platforms.
"""
import ast
import pathlib
import sys
from typing import List, Set
import pytest
IS_WINDOWS_PLATFORM = sys.platform == "win32"
REPO_ROOT = pathlib.Path(__file__).resolve().parent.parent
# The ONE file allowed to contain platform-specific code
ALLOWED_FILE = REPO_ROOT / "ouroboros" / "platform_layer.py"
# Directories to scan
SCAN_DIRS = [
REPO_ROOT / "ouroboros",
REPO_ROOT / "supervisor",
]
# Also scan server.py at the root
SCAN_FILES = [
REPO_ROOT / "server.py",
]
# ── Forbidden patterns ──────────────────────────────────────────────────
# Platform-specific modules that must not be imported outside platform_layer.py
FORBIDDEN_IMPORTS: Set[str] = {
"fcntl",
"msvcrt",
"winreg",
"resource",
}
# os.* calls that are platform-specific
FORBIDDEN_OS_ATTRS: Set[str] = {
"kill",
"killpg",
"setsid",
"getpgid",
}
# signal.* constants/calls that are platform-specific
FORBIDDEN_SIGNAL_ATTRS: Set[str] = {
"SIGKILL",
"SIGTERM",
}
def _collect_python_files() -> List[pathlib.Path]:
"""Collect all .py files to scan."""
files = []
for scan_dir in SCAN_DIRS:
if scan_dir.exists():
for py_file in scan_dir.rglob("*.py"):
if py_file.resolve() != ALLOWED_FILE.resolve():
files.append(py_file)
for f in SCAN_FILES:
if f.exists():
files.append(f)
return sorted(set(files))
def _scan_file(filepath: pathlib.Path) -> List[str]:
"""Scan a single file for platform-specific API violations.
Returns a list of violation descriptions.
"""
violations = []
try:
source = filepath.read_text(encoding="utf-8")
tree = ast.parse(source, filename=str(filepath))
except (SyntaxError, UnicodeDecodeError):
return violations
rel_path = str(filepath.relative_to(REPO_ROOT))
# Check top-level imports (not inside if/try/def/class)
for node in ast.iter_child_nodes(tree):
if isinstance(node, ast.Import):
for alias in node.names:
mod = alias.name.split(".")[0]
if mod in FORBIDDEN_IMPORTS:
violations.append(
f"{rel_path}:{node.lineno}: "
f"Top-level import of platform-specific module '{mod}'"
)
elif isinstance(node, ast.ImportFrom) and node.module:
top_mod = node.module.split(".")[0]
if top_mod in FORBIDDEN_IMPORTS:
violations.append(
f"{rel_path}:{node.lineno}: "
f"Top-level import from platform-specific module '{node.module}'"
)
# Check ALL attribute access for os.kill, signal.SIGKILL, etc.
for node in ast.walk(tree):
if isinstance(node, ast.Attribute) and isinstance(node.value, ast.Name):
if node.value.id == "os" and node.attr in FORBIDDEN_OS_ATTRS:
violations.append(
f"{rel_path}:{node.lineno}: "
f"Direct use of platform-specific os.{node.attr}"
)
if node.value.id == "signal" and node.attr in FORBIDDEN_SIGNAL_ATTRS:
violations.append(
f"{rel_path}:{node.lineno}: "
f"Direct use of platform-specific signal.{node.attr}"
)
# Check ImportFrom for direct imports like `from os import kill`
for node in ast.walk(tree):
if isinstance(node, ast.ImportFrom):
if node.module == "os":
for alias in node.names:
if alias.name in FORBIDDEN_OS_ATTRS:
violations.append(
f"{rel_path}:{node.lineno}: "
f"Direct import of platform-specific os.{alias.name}"
)
elif node.module == "signal":
for alias in node.names:
if alias.name in FORBIDDEN_SIGNAL_ATTRS:
violations.append(
f"{rel_path}:{node.lineno}: "
f"Direct import of platform-specific signal.{alias.name}"
)
return violations
def test_no_platform_specific_apis_outside_platform_layer():
"""All platform-specific API usage must be in platform_layer.py.
This test scans all .py files under ouroboros/ and supervisor/ (plus server.py)
for direct use of platform-specific APIs. Any usage outside of
ouroboros/platform_layer.py is a violation.
"""
all_violations = []
files = _collect_python_files()
for filepath in files:
all_violations.extend(_scan_file(filepath))
if all_violations:
report = "\n".join(f" {v}" for v in all_violations)
pytest.fail(
f"Found {len(all_violations)} platform-specific API violation(s) "
f"outside ouroboros/platform_layer.py:\n{report}\n\n"
f"All platform-specific code must go through platform_layer.py. "
f"See docs/DEVELOPMENT.md 'Platform Abstraction Rule'."
)
def test_platform_layer_exports_core_symbols():
"""platform_layer.py must export the core cross-platform symbols."""
from ouroboros.platform_layer import (
IS_WINDOWS,
IS_MACOS,
IS_LINUX,
)
# Smoke check: flags are booleans
assert isinstance(IS_WINDOWS, bool)
assert isinstance(IS_MACOS, bool)
assert isinstance(IS_LINUX, bool)
# Exactly one should be True (or none on exotic platforms)
assert sum([IS_WINDOWS, IS_MACOS, IS_LINUX]) <= 1
def test_normalize_repo_path_handles_windows_style_paths():
"""normalize_repo_path must handle Windows-style backslash paths on any OS.
Regression test for: on Linux/macOS PurePath does NOT convert backslashes,
so 'ouroboros\\\\tools\\\\registry.py' would bypass SAFETY_CRITICAL_PATHS
matching if we used PurePath without explicit backslash replacement.
(git.py protected-path matching routes through this SSOT.)
"""
from ouroboros.runtime_mode_policy import normalize_repo_path
# Windows-style safety-critical path must normalise to POSIX form
assert normalize_repo_path("ouroboros\\tools\\registry.py") == "ouroboros/tools/registry.py"
assert normalize_repo_path("ouroboros\\safety.py") == "ouroboros/safety.py"
assert normalize_repo_path("BIBLE.md") == "BIBLE.md"
# Mixed separators
assert normalize_repo_path("ouroboros/tools\\git.py") == "ouroboros/tools/git.py"
# Leading ./ stripped
assert normalize_repo_path("./ouroboros/safety.py") == "ouroboros/safety.py"
# Regular POSIX paths unaffected
assert normalize_repo_path("ouroboros/tools/registry.py") == "ouroboros/tools/registry.py"
@pytest.mark.skipif(not IS_WINDOWS_PLATFORM, reason="Windows-only")
def test_win32_overlapped_class_cached():
"""_win32_overlapped_class must return the same class object on every call.
ctypes rejects pointer arguments when the underlying Structure class differs
even if the layout is identical. If lock creates one OVERLAPPED class and
unlock creates another, UnlockFileEx will raise ctypes.ArgumentError.
"""
from ouroboros.platform_layer import _win32_overlapped_class
cls1 = _win32_overlapped_class()
cls2 = _win32_overlapped_class()
assert cls1 is cls2, (
"_win32_overlapped_class() returned different class objects — "
"this will cause ctypes.ArgumentError in unlock path"
)