Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 50 additions & 7 deletions .githooks/pre-commit
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import os
import subprocess
import sys
import time
import signal
from pathlib import Path

# Per-gate budget. 900s is the historical value (documented flakiness at 300s);
Expand All @@ -36,6 +37,49 @@ GATE_TIMEOUT = int(os.environ.get("MSCB_PRECOMMIT_GATE_TIMEOUT", "900"))
# global `subprocess` module — a global patch leaks into every other test in
# the same worker (CI: 14 unrelated failures).
Popen = subprocess.Popen
_RUN = subprocess.run


def _spawn_kwargs() -> dict:
"""A gate must be a process-group leader, so a timeout can kill its tree."""
if sys.platform == "win32":
return {"creationflags": getattr(subprocess, "CREATE_NO_WINDOW", 0)
| getattr(subprocess, "CREATE_NEW_PROCESS_GROUP", 0)}
return {"start_new_session": True}


def _kill_tree(proc) -> None:
"""Kill the gate and every descendant it spawned.

proc.kill() alone kills only the direct child. Observed 2026-11-14: a
timed-out commit left `verify_diary -> pytest tests/` running for 10+
minutes; the orphan then competed with the next run and made a fast test
look like an 11-minute hang. On Windows the tree needs taskkill /T; on
POSIX the child is a session leader, so killpg reaches the group.
"""
poll = getattr(proc, "poll", None)
if poll is not None and poll() is not None:
return
pid = getattr(proc, "pid", None)
if pid is not None:
try:
if sys.platform == "win32":
_RUN(["taskkill", "/PID", str(pid), "/T", "/F"],
stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL,
check=False, timeout=15)
else:
os.killpg(os.getpgid(pid), signal.SIGKILL)
except (OSError, subprocess.SubprocessError):
pass
try:
proc.kill()
except OSError:
pass
try:
proc.communicate(timeout=10)
except subprocess.TimeoutExpired:
pass



# §9 п.9 (ENCODING SAFETY): при выводе emoji-строк из stdout скриптов
Expand Down Expand Up @@ -84,20 +128,19 @@ def run_script(script_path: str, label: str, extra_args: list[str] | None = None
stderr=subprocess.DEVNULL,
encoding="utf-8",
errors="replace",
creationflags=getattr(subprocess, 'CREATE_NO_WINDOW', 0),
**_spawn_kwargs(),
)
# Таймаут-запас: verify_diary гоняет gate-zero (полный pytest ~108-130s под
# нагрузкой) — кап 120s давал флаки TimeoutExpired на коммитах (2026-08-08);
# 300→900 (2026-08-24): сюита выросла (live-sync + predict-наборы), 300s
# начал флакать при параллельной нагрузке.
# A timed-out gate is a verdict, not a claim about suite speed. The old
# "~108-130s" note stopped being true as the suite grew to 2528 tests, and
# on 2026-11-14 a loaded machine took 1092s for gate-zero. Keep the budget
# generous; the correctness requirement is that the tree is killed.
try:
stdout, _ = proc.communicate(timeout=GATE_TIMEOUT)
except subprocess.TimeoutExpired:
# Раньше это было необработанное исключение: хук умирал с трейсбеком и
# БЕЗ вердикта — его путали с «гейт сломан» и обходили через --no-verify.
# Таймаут — это вердикт: называем гейт, который не ответил.
proc.kill()
proc.communicate()
_kill_tree(proc)
print(f" ❌ {label}: TIMEOUT >{GATE_TIMEOUT}s — гейт не ответил, вердикта нет",
flush=True)
return False
Expand Down
11 changes: 11 additions & 0 deletions AGENT_DIARY.md
Original file line number Diff line number Diff line change
Expand Up @@ -1961,6 +1961,17 @@ BREACH: MSC-P-19 | date: 2026-10-03 | rule: §12 | guard_at_time: no | detected_
**4. Гипотеза «clean-state до фикса валидировал пустую установку» — опровергнута.** `scripts/verify_clean_state.sh` вообще не вызывает `install.py`: он сам создаёт venv и делает `pip install -e .`; `grep "install.py --yes"` по `.github/` и `scripts/` пуст. Значит баг `--yes` с v3.0 не влиял ни на один прогон CI — ровно поэтому его никто и не поймал; поймал только ручной прогон по README.
**Урок:** «CI зелёный» и «инструкция работает» — разные утверждения. Пока установщик не запускали как пользователь (по README, в пустом каталоге), его неинтерактивная ветка была мёртвой, а все гейты оставались зелёными.

## [2026-11-14] PR #133: suite-lock брался в каждом xdist-воркере

**Симптом:** CI на `fix/process-hardening` — `test (ubuntu)`, `test (windows)` и `clean-state` падали за ~6 секунд с `INTERNALERROR: worker crashed`, `crashitem` указывал на случайный тест (`test_deep_search.py::TestExtractKeyTerms::test_max_terms_limit`). Локально один прогон зелёный, `-n 4` — падает.

**Корень:** «один suite на дерево» было реализовано `@pytest.fixture(scope="session", autouse=True)`, а xdist выполняет session-фикстуры **в каждом воркере**. Один воркер брал файловую блокировку, остальные получали `None` → `pytest.exit(returncode=3)` → xdist видел «worker crashed». Маркер `MSCB_SUITE_LOCK_INHERITED` ставился в процессе-победителе и соседним воркерам не доставался.

**Fix:** блокировка перенесена в `pytest_configure` (контроллер берёт её до запуска воркеров) + `pytest_unconfigure` освобождает; воркеры выходят по `config.workerinput`, вложенные прогоны — по прежнему маркеру. Структурный guard в `tests/test_suite_lock.py`: `pytest_configure` есть, `workerinput` учитывается, фикстуры `_one_suite_per_tree` нет — на старом conftest краснеет.

**Проверка:** `pytest tests/test_deep_search.py -n 4` — 15 passed (было INTERNALERROR); полный suite `-n auto` — 2515 passed, 11 skipped, rc=0; структурный тест красный на старом conftest, зелёный на новом.

**Урок:** session-scoped фикстура в xdist — это N экземпляров, а не один; всё, что «на весь прогон» (блокировки, общие каталоги, счётчики), живёт в хуках контроллера.
## [2026-11-14] Установщик падал на шаге 7: `httpx`, `wmic` и «тихий» `--with-models`

**Симптом:** `python install.py` (интерпретатор `C:\Python314\python.exe`, без зависимостей проекта) — `⚠ Step 7/12: llama.cpp engine .. No module named 'httpx'`, дальше `[r]etry / [s]kip / [c]ancel`; итог прогона `11 ok, 1 skipped, 1351 s`.
Expand Down
17 changes: 14 additions & 3 deletions src/core/embedder/onnx_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,15 @@ def ensure_server_running(self) -> bool:
if self._health_check():
return True

# Межпоточный lock (на случай если несколько потоков одновременно вызывают)
with _client_lock:
# Межпоточный lock (на случай если несколько потоков одновременно вызывают).
# Ожидание ограничено: поток, который не может взять lock, честно отвечает
# False, а не висит вечно. Наблюдалось 2026-11-14: тест в полном прогоне
# простоял 654s на этом lock, хотя в одиночку занимает <0.01s — вечное
# ожидание превращало чужую утечку в «медленный тест» без вердикта.
if not _client_lock.acquire(timeout=_LOCK_TIMEOUT):
logger.error("[ONNX Client] launch lock busy for %.0fs; refusing to wait", _LOCK_TIMEOUT)
return False
try:
# Double-check под локом
if self._health_check():
return True
Expand All @@ -198,6 +205,8 @@ def ensure_server_running(self) -> bool:
return self._wait_for_server()
finally:
self._release_launch_mutex()
finally:
_client_lock.release()

def embed(self, text: str) -> List[float]:
"""Эмбеддинг одного текста."""
Expand Down Expand Up @@ -251,8 +260,10 @@ def shutdown(self):
pass


# Глобальный lock для координации внутри процесса
# Глобальный lock для координации внутри процесса. Ожидание ограничено:
# утёкший поток не должен подвешивать вызов навсегда (см. ensure_server_running).
_client_lock = threading.Lock()
_LOCK_TIMEOUT = 150.0 # launch + _wait_for_server(60s) + запас; вечность — не ответ

# Путь к корню проекта (репо или расширение Zed).
# parents[3]: src/core/embedder/onnx_client.py → корень.
Expand Down
84 changes: 84 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
в свой tmp_path, который pytest удаляет автоматически.
"""

import os
import sys
from pathlib import Path

Expand Down Expand Up @@ -80,3 +81,86 @@ def _fusion_default(monkeypatch):
"""
monkeypatch.delenv("MSCODEBASE_FUSION", raising=False)



# --- One suite per tree -------------------------------------------------------
# Observed 2026-11-14: a timed-out commit left `verify_diary -> pytest` running,
# then a second full suite started. The two competed for CPU; a test that takes
# <0.01s alone was reported at 654s, and the run looked like a hang. The OS
# releases this lock when the process dies, so a crash cannot leave it stale.

SUITE_LOCK = PROJECT_ROOT / ".pytest_cache" / "suite.lock"
_LOCK_INHERITED = "MSCB_SUITE_LOCK_INHERITED"


def _acquire_file_lock(path: Path):
path.parent.mkdir(parents=True, exist_ok=True)
handle = open(path, "a+b")
try:
handle.seek(0, os.SEEK_END)
if handle.tell() == 0:
handle.write(b"x")
handle.flush()
handle.seek(0)
if os.name == "nt":
import msvcrt
msvcrt.locking(handle.fileno(), msvcrt.LK_NBLCK, 1)
else:
import fcntl
fcntl.flock(handle.fileno(), fcntl.LOCK_EX | fcntl.LOCK_NB)
except OSError:
handle.close()
return None
return handle


def _release_file_lock(handle) -> None:
try:
handle.seek(0)
if os.name == "nt":
import msvcrt
msvcrt.locking(handle.fileno(), msvcrt.LK_UNLCK, 1)
else:
import fcntl
fcntl.flock(handle.fileno(), fcntl.LOCK_UN)
except OSError:
pass
finally:
handle.close()


def pytest_configure(config):
"""Refuse to start a second pytest on the same checkout.

Runs in the controller, not in a fixture: xdist executes session fixtures
inside **every** worker, so the fixture version had N workers racing for one
file lock — the losers called `pytest.exit(3)` and xdist reported
`INTERNALERROR: worker crashed` (CI 2026-11-14, ubuntu + windows, 3 of 4
workers). The controller acquires before workers spawn, and workers inherit
the marker through the environment.

A nested pytest spawned by a test inherits MSCB_SUITE_LOCK_INHERITED=1 and
skips the check, so `test_bootstrap_pipeline` still works.
"""
if os.environ.get(_LOCK_INHERITED) == "1":
return
if getattr(config, "workerinput", None) is not None: # xdist worker
return
handle = _acquire_file_lock(SUITE_LOCK)
if handle is None:
pytest.exit(
"Another pytest run already holds .pytest_cache/suite.lock. Two suites on "
"one tree make timings meaningless and can race on shared temp data. Wait "
"for it to finish; if no run is active, delete the file and retry.",
returncode=3,
)
config._mscb_suite_lock = handle
os.environ[_LOCK_INHERITED] = "1"


def pytest_unconfigure(config):
handle = getattr(config, "_mscb_suite_lock", None)
if handle is None:
return
os.environ.pop(_LOCK_INHERITED, None)
_release_file_lock(handle)
37 changes: 37 additions & 0 deletions tests/test_idle_reload.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,25 @@
import threading
from unittest.mock import patch

import pytest

from src.core.embedder.onnx_client import OnnxEmbedderClient


@pytest.fixture(autouse=True)
def _fresh_client_lock(monkeypatch):
"""Каждый тест получает свой lock.

`_client_lock` — модульный глобал. Поток, оставшийся от другого теста,
держал его; после появления таймаута лока (2026-11-14) `ensure_server_running`
честно возвращал False вместо зависания, и тест падал по чужому состоянию
(clean-state, PR #133). Изоляция убирает порядок тестов из уравнения.
"""
from src.core.embedder import onnx_client as mod

monkeypatch.setattr(mod, "_client_lock", threading.Lock())


def _client():
# Свободный порт: реальный health-check обязан вернуть False
return OnnxEmbedderClient(port=19999, model_name="test-model")
Expand Down Expand Up @@ -136,3 +152,24 @@ def worker():
assert not errors
assert launch.call_count == 1, "Сервер должен запускаться ровно один раз"
assert all(results), "Все потоки должны получить успех"

def test_ensure_server_running_gives_up_when_the_launch_lock_is_held(monkeypatch):
"""A leaked thread must not turn the next call into an indefinite wait.

This file takes 2.7s alone; in the 2026-11-14 full run one test held
_client_lock and another waited 654s on it. The lock now has a timeout.
"""
import time

from src.core.embedder import onnx_client as mod

c = _client()
monkeypatch.setattr(mod, "_LOCK_TIMEOUT", 0.05)
assert mod._client_lock.acquire(timeout=1.0)
try:
with patch.object(c, "_health_check", return_value=False):
started = time.monotonic()
assert c.ensure_server_running() is False
assert time.monotonic() - started < 5
finally:
mod._client_lock.release()
50 changes: 50 additions & 0 deletions tests/test_precommit_hook_contract.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,3 +142,53 @@ def test_fixed_hook_answers_the_same_timeout(tmp_path, capsys):
assert "TIMEOUT" in out and "slowgate" in out
assert "Traceback" not in out


# --- process tree on timeout -------------------------------------------------

def test_gate_is_spawned_as_a_process_group_leader():
"""A gate that is not a group leader cannot be killed with its children."""
mod = _load_hook(HOOK, None, "hook_group")
kw = mod._spawn_kwargs()
if sys.platform == "win32":
flag = getattr(subprocess, "CREATE_NEW_PROCESS_GROUP", 0)
assert kw["creationflags"] & flag
else:
assert kw["start_new_session"] is True


def test_timeout_kills_the_process_tree(tmp_path, monkeypatch):
"""proc.kill() alone left `verify_diary -> pytest` alive for 10+ minutes."""
mod = _load_hook(HOOK, 1, "hook_tree")
gate = tmp_path / "gate.py"
gate.write_text("print('ok')\n", encoding="utf-8")
calls = []

class Proc:
pid = 4321

def poll(self):
return None

def kill(self):
calls.append("kill")

def communicate(self, timeout=None):
if "raised" not in calls:
calls.append("raised")
raise subprocess.TimeoutExpired(cmd="gate", timeout=timeout)
return ("", None)

mod.Popen = lambda *a, **k: Proc()
if sys.platform == "win32":
monkeypatch.setattr(mod, "_RUN", lambda cmd, **kw: calls.append(("run", list(cmd))))
else:
monkeypatch.setattr(mod.os, "getpgid", lambda pid: 7)
monkeypatch.setattr(mod.os, "killpg", lambda pgid, sig: calls.append(("killpg", pgid)))

assert mod.run_script(str(gate), "gate") is False
if sys.platform == "win32":
tree = next(c for c in calls if isinstance(c, tuple) and c[0] == "run")[1]
assert tree[0] == "taskkill" and "/T" in tree and "/F" in tree
else:
assert ("killpg", 7) in calls
assert "kill" in calls, "the direct child must still be killed as a fallback"
51 changes: 51 additions & 0 deletions tests/test_suite_lock.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
"""The suite lock must refuse a second run and free itself on release.

Two full suites on one checkout are what turned a <0.01s test into a 654s
report on 2026-11-14. The lock is a file lock, so the OS frees it if a run dies.
"""

from __future__ import annotations

import importlib.util
from pathlib import Path

ROOT = Path(__file__).resolve().parents[1]


def _conftest():
spec = importlib.util.spec_from_file_location("mscb_conftest", ROOT / "tests" / "conftest.py")
module = importlib.util.module_from_spec(spec)
spec.loader.exec_module(module)
return module


def test_second_acquire_fails_and_release_frees(tmp_path: Path) -> None:
mod = _conftest()
lock = tmp_path / "suite.lock"

first = mod._acquire_file_lock(lock)
assert first is not None
try:
assert mod._acquire_file_lock(lock) is None
finally:
mod._release_file_lock(first)

second = mod._acquire_file_lock(lock)
assert second is not None
mod._release_file_lock(second)


def test_lock_is_taken_in_the_controller_not_in_a_session_fixture():
"""Structural guard for the 2026-11-14 crash.

A session-scoped fixture runs inside every xdist worker; the losers called
pytest.exit(3) and CI reported `INTERNALERROR: worker crashed`. From inside
the suite this is invisible (a nested run inherits the marker), so the shape
is asserted: the lock lives in pytest_configure, workers skip via
workerinput, and the per-worker fixture is gone.
"""
source = (ROOT / "tests" / "conftest.py").read_text(encoding="utf-8")
assert "def pytest_configure(config)" in source
assert "def pytest_unconfigure(config)" in source
assert 'getattr(config, "workerinput", None)' in source
assert "_one_suite_per_tree" not in source
Loading