From 75284fa13e679862e27912390285dbb4aadf7157 Mon Sep 17 00:00:00 2001 From: Chris Dare Date: Sat, 19 Sep 2026 17:45:04 -0400 Subject: [PATCH] fix(deps): a PEP 735 dev group, so `uv run` stops stripping the toolchain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The call-site half of this landed already: tests/test_daily_metrics_report.py uses sys.executable rather than `uv run`. The other half did not, and the defect it closes is still live on main -- it just is not reachable from the suite any more. pytest and ruff are declared ONLY in [project.optional-dependencies].dev. uv installs GROUPS by default and extras NEVER, so any `uv run` fired from inside this repository syncs the project venv down to the default dependency set and prunes them out of it. Measured on this box against the real .venv, `uv sync --dry-run`: before would uninstall 9: pytest, ruff, iniconfig, pluggy, arxmcp, pyinstaller, pyinstaller-hooks-contrib, pefile, altgraph, pywin32-ctypes after would uninstall 6: arxmcp + the PyInstaller stack only pytest, ruff, iniconfig and pluggy are preserved. The residual is a different thing and deliberately not addressed here: the PyInstaller build stack is installed from requirements-build.txt, kept OUTSIDE pyproject/uv.lock by apps/desktop/pyinstaller/desktop_package.py so the build venv is throwaway, so uv correctly regards those as foreign to the project environment. This matters because roughly twenty `uv run python -m tools.…` commands are documented across tools/ docstrings and docs/ops/ runbooks. The failure mode is the nasty kind: the in-flight command survives on already-imported modules and appears to succeed, and the NEXT `make test` dies with "No module named pytest". The duplication between the extra and the group is load-bearing rather than an oversight. `pip install -e ".[dev]"` (Makefile bootstrap, docs/install.md) reads the extra; `uv sync` reads the group; neither table can reference the other. tests/test_dev_toolchain_guard.py fails if they drift apart. GUARD NARROWED, AND WHY. The guard as originally written banned every string constant naming the uv binary anywhere under tests/. On current main that flags tests/test_desktop_package.py:512, which is `shutil.which("uv")` -- a PATH lookup that starts no process, syncs nothing and cannot prune a package. It mirrors the precondition the packaging driver itself checks. Editing working packaging code to satisfy an over-broad detector would be the wrong direction, so the detector now exempts arguments to `which()` while still catching argv lists, Windows `uv.exe` and hardcoded absolute paths. Weakening a guard is only safe if the weakening is itself tested, so TestDetectorItself drives the detector over synthetic source: presence checks pass, invocation forms are still caught. One of those tests records a LIMIT rather than a capability -- a `which()` result bound to a variable and then passed to `run()` is invisible to any string-constant detector, with or without the exemption -- so the guard does not imply cover it does not have. uv.lock regenerated. Beyond the dev-group rows it also picks up the `contract` extra (jsonschema + mfc), which pyproject declares and the lockfile did not carry at all: pre-existing drift, repaired here because leaving it would mean knowingly committing a lock that does not match its manifest. No pin changes. Suite: 5406 passed, 188 skipped, 1 xfailed, 20 failed. All 20 failures are pre-existing on origin/main and none are in the changed surface -- 16 in test_formal_release_pin.py (`No module named 'mfc'`), 3 in test_desktop_package.py (WinError 1314, symlink creation needs Developer Mode), 1 in test_textbook_parser.py (`signal.SIGKILL` does not exist on Windows; ingest/textbook_parser.py:551 is byte-identical to origin/main). Ruff clean. NOT FIXED HERE, and it is environmental rather than code: `uv lock` cannot run on this machine without an explicit --python. C:\Users\cedar\bin\python3.exe is a stub with no python.exe beside it, no pyvenv.cfg and no stdlib; uv selects it because that directory is early on PATH, and any build backend that spawns the sibling interpreter dies with `No Python at '"C:\Users\cedar\bin\python.exe'` -- note the mangled quote. That stub is also what made this same test fail on the July branch. No code change fixes it. --- pyproject.toml | 31 +++++ tests/test_dev_toolchain_guard.py | 223 ++++++++++++++++++++++++++++++ uv.lock | 28 +++- 3 files changed, 281 insertions(+), 1 deletion(-) create mode 100644 tests/test_dev_toolchain_guard.py diff --git a/pyproject.toml b/pyproject.toml index 21e63adf..86fe5e77 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -355,6 +355,37 @@ contract = [ # imports the mineru package directly. # See docs/install.md for the install procedure. +[dependency-groups] +# PEP 735 dependency group — deliberately MIRRORS the ``dev`` extra above. +# +# The duplication is load-bearing, not an oversight. The two tables are +# consumed by different installers and neither can reference the other: +# +# * ``[project.optional-dependencies].dev`` is what ``pip install -e +# ".[dev]"`` resolves — the Makefile ``bootstrap`` target and +# docs/install.md both use that form. +# * ``[dependency-groups].dev`` is what **uv** installs BY DEFAULT on +# every ``uv sync`` and every ``uv run``. Extras are NOT installed by +# default; groups named in ``default-groups`` are, and ``dev`` is +# uv's implicit default. +# +# Without this block, ANY ``uv run`` fired from inside the repo syncs the +# project ``.venv`` down to the default dependency set and PRUNES pytest +# and ruff out of it — including a ``uv run`` fired from inside the test +# suite, which strips the toolchain of the very interpreter running the +# tests. Observed live on 2026-08-01 (tests/test_daily_metrics_report.py +# ``TestRegenFixture``): the in-flight run survived on already-imported +# modules, so the test passed, and the NEXT ``make test`` died with +# "No module named pytest". Roughly twenty ``uv run python -m tools.…`` +# commands are documented across tools/ docstrings and docs/ops/ runbooks; +# this block is what makes every one of them safe to run. +# +# ``tests/test_dev_toolchain_guard.py`` fails if the two lists drift apart. +dev = [ + "ruff>=0.5", + "pytest>=8.0", +] + [tool.ruff] line-length = 100 target-version = "py311" diff --git a/tests/test_dev_toolchain_guard.py b/tests/test_dev_toolchain_guard.py new file mode 100644 index 00000000..fc422935 --- /dev/null +++ b/tests/test_dev_toolchain_guard.py @@ -0,0 +1,223 @@ +"""Guards that the test suite cannot destroy its own toolchain. + +Two defects were observed live on 2026-08-01 in +``tests/test_daily_metrics_report.py::TestRegenFixture``, which shelled +out to ``uv run python -c "…"`` to render a fixture in a fresh process. +Neither had anything to do with product code: + +1. **``uv run`` prunes the test runner's own tooling.** ``uv run`` syncs + the project environment to the project's DEFAULT dependency set before + executing. ``pytest`` and ``ruff`` were declared ONLY in the ``dev`` + *extra*, and uv does not install extras by default — so the subprocess + uninstalled pytest, ruff and pip from the very ``.venv`` the running + pytest process had been launched from. The in-flight run survived on + already-imported modules, so **the test passed** while the NEXT + ``make test`` died with "No module named pytest". + +2. **On Windows the same call hard-fails when an MCP session is live.** + While syncing, uv replaces the project's own installed entry points, + including ``.venv/Scripts/arxmcp-shim.exe``. Any Claude session with + the arXMCP MCP server connected holds that exe open, Windows refuses + the delete, and uv aborts with ``Access is denied. (os error 5)``. + Pruning happens FIRST, so this path leaves a half-stripped venv AND a + red test. + +Both are closed here, from two directions: + +* ``TestUvRunNotInvokedFromTests`` bans naming the ``uv`` binary from + anywhere under ``tests/``. The fixed call site uses ``sys.executable``, + which is already the correct project interpreter under pytest and needs + no environment sync at all. This also retires the hardcoded + ``/Users/chris.dare/Library/Python/3.9/bin/uv`` fallback the old call + site carried — dead on every non-macOS box, and user-specific even + there (the ``docs/ops/`` peer of this rule lives in + ``tests/test_runbook_index.py::TestNoHardcodedUserPaths``). + +* ``TestDevGroupMirrorsDevExtra`` pins the ``[dependency-groups].dev`` + block in ``pyproject.toml`` that makes ``uv run`` non-destructive + everywhere else in the repo — roughly twenty ``uv run python -m + tools.…`` commands are documented across tools/ docstrings and + docs/ops/ runbooks, and every one of them used to strip the toolchain. +""" + +from __future__ import annotations + +import ast +import tomllib +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parents[1] +PYPROJECT = REPO_ROOT / "pyproject.toml" +TESTS_DIR = REPO_ROOT / "tests" + + +def _load_pyproject() -> dict: + with PYPROJECT.open("rb") as fh: + return tomllib.load(fh) + + +class TestDevGroupMirrorsDevExtra: + """``uv`` installs dependency GROUPS by default and extras never. + + ``pip install -e ".[dev]"`` (Makefile ``bootstrap``, docs/install.md) + reads the extra; ``uv sync`` / ``uv run`` read the group. Neither + table can reference the other, so pyproject.toml carries both and + this test is what keeps them from drifting. + """ + + def test_dev_group_exists(self) -> None: + groups = _load_pyproject().get("dependency-groups", {}) + assert "dev" in groups, ( + "pyproject.toml has no `[dependency-groups].dev`. Without it " + "every `uv run` in this repo syncs the project .venv down to " + "the default dependency set and PRUNES pytest + ruff out of " + "it — including a `uv run` fired from inside the suite, which " + "strips the interpreter running the tests. Re-add the block." + ) + + def test_dev_group_matches_dev_extra(self) -> None: + cfg = _load_pyproject() + extra = cfg["project"]["optional-dependencies"]["dev"] + group = cfg["dependency-groups"]["dev"] + # Only plain requirement strings are compared. PEP 735 also allows + # `{include-group = ...}` table entries; if one is ever added here + # this guard must be revisited rather than silently loosened. + assert all(isinstance(x, str) for x in group), ( + "`[dependency-groups].dev` gained a non-string entry " + f"({group!r}). This guard compares requirement strings only — " + "update it deliberately." + ) + assert sorted(group) == sorted(extra), ( + "`[dependency-groups].dev` and " + "`[project.optional-dependencies].dev` have drifted:\n" + f" group (uv sync / uv run): {sorted(group)}\n" + f" extra (pip install -e '.[dev]'): {sorted(extra)}\n" + "Both must list the full dev toolchain. A package present in " + "only the extra is invisible to `uv run` and gets pruned from " + "the project .venv on the next sync." + ) + + +class TestUvRunNotInvokedFromTests: + """No test may shell out to the ``uv`` binary. + + A test that needs a fresh interpreter wants ``sys.executable``, not + an environment sync. Detection is AST-based (string CONSTANTS only), + so the prose in this file — and in the fixed call site's explanatory + comment — does not trip it; only real code naming the binary does. + """ + + @staticmethod + def _names_uv_binary(value: str) -> bool: + # Bare executable name, Windows form, or any absolute/relative + # path ending in the binary (catches the old hardcoded + # `/Users/chris.dare/Library/Python/3.9/bin/uv` fallback). + stem = value.replace("\\", "/").rsplit("/", 1)[-1] + return stem in {"uv", "uv.exe"} + + @staticmethod + def _presence_check_constants(tree: ast.AST) -> set[int]: + """Constants that are an argument to ``shutil.which`` / ``which``. + + A PATH lookup is not an invocation: it starts no process, syncs no + environment and cannot prune a package. Banning it would push callers + into running the binary blind to decide whether it exists, which is + strictly worse. What stays banned is naming the binary anywhere it + could be EXECUTED -- an argv[0], a command list, a hardcoded path. + """ + allowed: set[int] = set() + for node in ast.walk(tree): + if not isinstance(node, ast.Call): + continue + func = node.func + name = ( + func.attr if isinstance(func, ast.Attribute) + else func.id if isinstance(func, ast.Name) + else None + ) + if name == "which": + for arg in node.args: + if isinstance(arg, ast.Constant): + allowed.add(id(arg)) + return allowed + + def test_no_uv_binary_reference_in_tests(self) -> None: + offenders: list[str] = [] + # This file is the detector; its own literals are the pattern + # being banned, not an instance of it. + self_path = Path(__file__).resolve() + for py in sorted(TESTS_DIR.rglob("*.py")): + if py.resolve() == self_path: + continue + try: + tree = ast.parse(py.read_text(encoding="utf-8"), filename=str(py)) + except SyntaxError: # pragma: no cover - would fail collection anyway + continue + allowed = self._presence_check_constants(tree) + for node in ast.walk(tree): + if ( + isinstance(node, ast.Constant) + and isinstance(node.value, str) + and self._names_uv_binary(node.value) + and id(node) not in allowed + ): + offenders.append( + f" {py.relative_to(REPO_ROOT).as_posix()}:" + f"{node.lineno}: {node.value!r}" + ) + assert not offenders, ( + "Test code names the `uv` binary:\n" + + "\n".join(offenders) + + "\n\n`uv run` syncs the project environment before executing " + "and uninstalls anything outside the default dependency set — " + "run from inside the suite it prunes pytest/ruff out of the " + "venv the tests are running in, and on Windows it additionally " + "fails outright on the locked arxmcp-shim.exe when an MCP " + "session is connected.\nUse `sys.executable` for a fresh " + "interpreter (set PYTHONPATH=REPO_ROOT so the repo imports), " + "or drive the real thing through tools/wheel_install_check.py, " + "which builds into a throwaway venv and never touches .venv." + ) + + +class TestDetectorItself: + """The detector was narrowed to permit ``shutil.which("uv")``. That + narrowing is only safe if it did NOT also stop catching invocation, so + the detector is exercised against synthetic source rather than trusted. + + Without this, a future "just allow one more form" edit could hollow the + guard out while every other test in this file still passes. + """ + + @staticmethod + def _offenders(source: str) -> list[str]: + tree = ast.parse(source) + allowed = TestUvRunNotInvokedFromTests._presence_check_constants(tree) + return [ + node.value + for node in ast.walk(tree) + if isinstance(node, ast.Constant) + and isinstance(node.value, str) + and TestUvRunNotInvokedFromTests._names_uv_binary(node.value) + and id(node) not in allowed + ] + + def test_presence_check_is_permitted(self) -> None: + assert self._offenders('import shutil; shutil.which("uv")') == [] + assert self._offenders('from shutil import which; which("uv")') == [] + + def test_invocation_is_still_caught(self) -> None: + """The forms that actually sync the environment.""" + assert self._offenders('subprocess.run(["uv", "run", "python"])') == ["uv"] + assert self._offenders('subprocess.run(["uv.exe", "sync"])') == ["uv.exe"] + assert self._offenders('CMD = "/opt/homebrew/bin/uv"') == ["/opt/homebrew/bin/uv"] + + def test_a_which_result_that_is_then_run_is_not_laundered(self) -> None: + """``shutil.which`` returns a PATH; passing that VARIABLE to ``run()`` + is invisible to a string-constant detector with or without the + exemption. The exemption covers the literal argument only -- it does + not make such a call site safe. This test records the limit rather + than letting the guard imply cover it does not have. + """ + src = 'uv = shutil.which("uv"); subprocess.run([uv, "run"])' + assert self._offenders(src) == [] diff --git a/uv.lock b/uv.lock index 8f1fb14b..9cdc00bb 100644 --- a/uv.lock +++ b/uv.lock @@ -71,6 +71,16 @@ dependencies = [ ] [package.optional-dependencies] +contract = [ + { name = "jsonschema" }, + { name = "mfc" }, +] +dev = [ + { name = "pytest" }, + { name = "ruff" }, +] + +[package.dev-dependencies] dev = [ { name = "pytest" }, { name = "ruff" }, @@ -83,10 +93,12 @@ requires-dist = [ { name = "faiss-cpu", specifier = ">=1.7" }, { name = "fastapi", specifier = ">=0.115" }, { name = "jinja2", specifier = ">=3.1.3" }, + { name = "jsonschema", marker = "extra == 'contract'", specifier = ">=4.18" }, { name = "kuzu", specifier = "==0.11.3" }, { name = "lancedb", specifier = ">=0.30.2,<0.35" }, { name = "latex2mathml", specifier = "==3.81.0" }, { name = "mcp", specifier = ">=1.27,<2" }, + { name = "mfc", marker = "extra == 'contract'", git = "https://github.com/chris-dare-dev/math-formal-contract-lean?subdirectory=contract&rev=b5a63cbab0cc2625cdfeacadfa9cb4b64a419598" }, { name = "numpy", specifier = ">=1.24" }, { name = "opentelemetry-api", specifier = ">=1.41,<2" }, { name = "opentelemetry-exporter-otlp-proto-grpc", specifier = ">=1.41,<2" }, @@ -106,7 +118,13 @@ requires-dist = [ { name = "uvicorn", extras = ["standard"], specifier = ">=0.30" }, { name = "zss", specifier = ">=1.2.0" }, ] -provides-extras = ["dev"] +provides-extras = ["dev", "contract"] + +[package.metadata.requires-dev] +dev = [ + { name = "pytest", specifier = ">=8.0" }, + { name = "ruff", specifier = ">=0.5" }, +] [[package]] name = "attrs" @@ -915,6 +933,14 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/b3/38/89ba8ad64ae25be8de66a6d463314cf1eb366222074cfda9ee839c56a4b4/mdurl-0.1.2-py3-none-any.whl", hash = "sha256:84008a41e51615a49fc9966191ff91509e3c40b939176e643fd50a5c2196b8f8", size = 9979, upload-time = "2022-08-14T12:40:09.779Z" }, ] +[[package]] +name = "mfc" +version = "0.1.0" +source = { git = "https://github.com/chris-dare-dev/math-formal-contract-lean?subdirectory=contract&rev=b5a63cbab0cc2625cdfeacadfa9cb4b64a419598#b5a63cbab0cc2625cdfeacadfa9cb4b64a419598" } +dependencies = [ + { name = "jsonschema" }, +] + [[package]] name = "mpmath" version = "1.3.0"