ouroboros/tests/test_ws2_skill_dispatch.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

46 lines
1.5 KiB
Python

"""WS2 — skill-dispatch resilience (v6.34.0).
A1: the ctx calling-convention is decided on the RAW handler (the runtime wrapper
is (*args, **kwargs), so inspecting it always forces a ctx-first call → TypeError
for keyword-only / zero-arg handlers like unix_computer_use wait/capabilities).
(A2's proposed _execution_lock skip-for-no-deps was withdrawn: it reopened the
cross-skill dependency-leak the lock guards — see test_extension_isolated_deps.)
"""
from __future__ import annotations
import functools
import inspect
def test_handler_wants_ctx_raw_vs_wrapper():
from ouroboros.extension_process_runner import _handler_wants_ctx
# ctx-less handlers must NOT receive ctx.
assert _handler_wants_ctx(lambda: None) is False
assert _handler_wants_ctx(lambda *, ms=500: None) is False
def kwonly(*, x=1):
return x
assert _handler_wants_ctx(kwonly) is False
# ctx-first handlers DO receive ctx.
def ctxfn(ctx, x=1):
return x
assert _handler_wants_ctx(ctxfn) is True
# The bug: a naked (*args, **kwargs) runtime wrapper reports ctx-wanted...
def naked_wrapper(*args, **kwargs):
return None
assert _handler_wants_ctx(naked_wrapper) is True
# ...the fix: a functools.wraps wrapper exposes __wrapped__ so inspect.unwrap
# recovers the real ctx-less signature (dispatch's legacy fallback).
@functools.wraps(kwonly)
def wrapped(*args, **kwargs):
return kwonly(**kwargs)
assert _handler_wants_ctx(inspect.unwrap(wrapped)) is False