diff --git a/Cargo.lock b/Cargo.lock index 4a2e8cc..a4b85d7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -40,7 +40,7 @@ checksum = "e9d4ee0d472d1cd2e28c97dfa124b3d8d992e10eb0a035f33f5d12e3a177ba3b" [[package]] name = "amplifier-core" -version = "1.4.0" +version = "1.4.1" dependencies = [ "chrono", "log", @@ -76,7 +76,7 @@ dependencies = [ [[package]] name = "amplifier-core-py" -version = "1.4.0" +version = "1.4.1" dependencies = [ "amplifier-core", "log", diff --git a/bindings/python/Cargo.toml b/bindings/python/Cargo.toml index b43bb85..01a0059 100644 --- a/bindings/python/Cargo.toml +++ b/bindings/python/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "amplifier-core-py" -version = "1.4.0" +version = "1.4.1" edition = "2021" description = "PyO3 bridge for amplifier-core Rust kernel" license = "MIT" diff --git a/context/release-mandate.md b/context/release-mandate.md index 642401d..2887c54 100644 --- a/context/release-mandate.md +++ b/context/release-mandate.md @@ -165,3 +165,4 @@ there is no fast local rollback — users must wait for a fix. | v1.2.3 | 2026-03-16 | `session_state` crash — missing dict field on RustCoordinator | CLI startup crashed | Yanked | | v1.2.4 | 2026-03-16 | `_tool_dispatch_context` crash — RustCoordinator lacked `__dict__` | All tool dispatch crashed | Yanked | | v1.2.4 | 2026-03-16 | Version files not bumped before tagging | PyPI publish rejected (400) | Re-tagged | +| v1.4.0 | 2026-04-25 | Validator imports leaked pytest as a runtime dep — `validation/structural/__init__.py` eagerly loaded test base classes whose top-level `import pytest` is undeclared in `pyproject.toml`. The 5 type validators imported `check_on_session_ready` from `.structural` (not `.base`), so a clean `pip install amplifier-core` failed `amplifier` startup with `ModuleNotFoundError: No module named 'pytest'`. The smoke test missed it because the pre-existing CLI install (Step 4) pulled pytest as a transitive dep, masking the missing runtime declaration. | All clean-env users hit ImportError at session init | Yanked. Hot-fix in v1.4.1: moved `check_on_session_ready` to `validation/base.py`, updated 5 validator imports, added pristine-import regression test (subprocess + `sys.modules` poisoning) and pristine-import preflight (Step 1b) in `e2e-smoke-test.sh` that imports the wheel into a bare `python:3.12-slim` before any deps pollute the env. | diff --git a/crates/amplifier-core/Cargo.toml b/crates/amplifier-core/Cargo.toml index b052124..ba61d61 100644 --- a/crates/amplifier-core/Cargo.toml +++ b/crates/amplifier-core/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "amplifier-core" -version = "1.4.0" +version = "1.4.1" edition = "2021" description = "Pure Rust kernel for the Amplifier modular AI agent system" license = "MIT" diff --git a/pyproject.toml b/pyproject.toml index 550e2e2..30bcf95 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "amplifier-core" -version = "1.4.0" +version = "1.4.1" description = "Rust kernel with Python bindings for the Amplifier modular AI agent framework" license = "MIT" readme = "README.md" diff --git a/python/amplifier_core/validation/base.py b/python/amplifier_core/validation/base.py index 7336cf1..6a3cbdc 100644 --- a/python/amplifier_core/validation/base.py +++ b/python/amplifier_core/validation/base.py @@ -1,11 +1,20 @@ """ Base types for module validation. -Provides ValidationCheck and ValidationResult dataclasses used by all validators. +Provides ValidationCheck and ValidationResult dataclasses used by all validators, +plus structural-check helper functions that operate on imported module objects +without depending on the test-class hierarchy in ``validation.structural``. + +NOTE: ``check_on_session_ready`` lives here (not in ``validation.structural``) +so the per-type validators can import it without pulling in the pytest-dependent +test base classes at module-load time. See incident #5 in +``context/release-mandate.md`` for the v1.4.0 regression that motivated this. """ +import inspect from dataclasses import dataclass from dataclasses import field +from typing import Any from typing import Literal @@ -51,3 +60,69 @@ def summary(self) -> str: passed_count = sum(1 for c in self.checks if c.passed) status = "PASSED" if self.passed else "FAILED" return f"{status}: {passed_count}/{len(self.checks)} checks passed ({len(self.errors)} errors, {len(self.warnings)} warnings)" + + +def check_on_session_ready(module: Any) -> ValidationCheck | None: + """Check whether a module's on_session_ready() function, if present, is valid. + + Validates: + 1. Presence: returns None when on_session_ready is absent (no check needed). + 2. Async: returns a failing ValidationCheck when on_session_ready exists but + is not async (must be ``async def``). + 3. Arity (B5): returns a failing ValidationCheck when on_session_ready exists, + is async, but accepts no positional arguments — the coordinator argument + is required. + + Args: + module: The imported module object to inspect. + + Returns: + None if no issue found, or a ValidationCheck with passed=False describing + the first problem encountered. + + Note: + This function lives in ``validation.base`` (not ``validation.structural``) + so that the per-type validators can import it without triggering the + pytest-dependent test base classes in ``validation.structural``. See + incident #5 in ``context/release-mandate.md`` for the v1.4.0 regression + that motivated this placement. + """ + fn = getattr(module, "on_session_ready", None) + if fn is None: + return None + if not inspect.iscoroutinefunction(fn): + return ValidationCheck( + name="on_session_ready_async", + passed=False, + message=( + "on_session_ready() must be async: found sync function. " + "Use 'async def on_session_ready(coordinator) -> None:'" + ), + severity="error", + ) + # B5 fix: validate arity — must accept at least one positional arg (coordinator) + try: + sig = inspect.signature(fn) + positional_params = [ + p + for p in sig.parameters.values() + if p.kind + in ( + inspect.Parameter.POSITIONAL_OR_KEYWORD, + inspect.Parameter.POSITIONAL_ONLY, + ) + and p.default is inspect.Parameter.empty + ] + if len(positional_params) < 1: + return ValidationCheck( + name="on_session_ready_async", + passed=False, + message=( + "on_session_ready() must accept a coordinator argument: " + "async def on_session_ready(coordinator) -> None" + ), + severity="error", + ) + except (ValueError, TypeError): + pass # Can't inspect — let it pass; runtime will catch it + return None diff --git a/python/amplifier_core/validation/context.py b/python/amplifier_core/validation/context.py index ded5115..5440783 100644 --- a/python/amplifier_core/validation/context.py +++ b/python/amplifier_core/validation/context.py @@ -15,7 +15,7 @@ from .base import ValidationCheck from .base import ValidationResult -from .structural import check_on_session_ready +from .base import check_on_session_ready def _implements_context_manager_interface(obj: Any) -> bool: diff --git a/python/amplifier_core/validation/hook.py b/python/amplifier_core/validation/hook.py index fcce8a9..e3e68ee 100644 --- a/python/amplifier_core/validation/hook.py +++ b/python/amplifier_core/validation/hook.py @@ -15,7 +15,7 @@ from .base import ValidationCheck from .base import ValidationResult -from .structural import check_on_session_ready +from .base import check_on_session_ready def _implements_hook_handler_interface(obj: Any) -> bool: diff --git a/python/amplifier_core/validation/orchestrator.py b/python/amplifier_core/validation/orchestrator.py index 1cf6a22..09a6e98 100644 --- a/python/amplifier_core/validation/orchestrator.py +++ b/python/amplifier_core/validation/orchestrator.py @@ -15,7 +15,7 @@ from .base import ValidationCheck from .base import ValidationResult -from .structural import check_on_session_ready +from .base import check_on_session_ready def _implements_orchestrator_interface(obj: Any) -> bool: diff --git a/python/amplifier_core/validation/provider.py b/python/amplifier_core/validation/provider.py index e787f15..3626f8a 100644 --- a/python/amplifier_core/validation/provider.py +++ b/python/amplifier_core/validation/provider.py @@ -16,7 +16,7 @@ from ..models import ProviderInfo from .base import ValidationCheck from .base import ValidationResult -from .structural import check_on_session_ready +from .base import check_on_session_ready def _implements_provider_interface(obj: Any) -> bool: diff --git a/python/amplifier_core/validation/structural/__init__.py b/python/amplifier_core/validation/structural/__init__.py index c65e3e8..8f8760e 100644 --- a/python/amplifier_core/validation/structural/__init__.py +++ b/python/amplifier_core/validation/structural/__init__.py @@ -30,10 +30,12 @@ class TestMyToolStructural(ToolStructuralTests): - No duplication: Modules just inherit, no copy-paste """ -import inspect -from typing import Any - -from ..base import ValidationCheck +# Re-export ``check_on_session_ready`` from ``validation.base`` for backward +# compatibility. The function lives in ``base`` (not here) so that the per-type +# validators can import it without triggering the pytest-dependent test base +# classes below. See incident #5 in ``context/release-mandate.md`` for the +# v1.4.0 regression that motivated this placement. +from ..base import check_on_session_ready from .test_context import ContextStructuralTests from .test_hook import HookStructuralTests from .test_orchestrator import OrchestratorStructuralTests @@ -48,62 +50,3 @@ class TestMyToolStructural(ToolStructuralTests): "ContextStructuralTests", "check_on_session_ready", ] - - -def check_on_session_ready(module: Any) -> ValidationCheck | None: - """Check whether a module's on_session_ready() function, if present, is valid. - - Validates: - 1. Presence: returns None when on_session_ready is absent (no check needed). - 2. Async: returns a failing ValidationCheck when on_session_ready exists but - is not async (must be ``async def``). - 3. Arity (B5): returns a failing ValidationCheck when on_session_ready exists, - is async, but accepts no positional arguments — the coordinator argument - is required. - - Args: - module: The imported module object to inspect. - - Returns: - None if no issue found, or a ValidationCheck with passed=False describing - the first problem encountered. - """ - fn = getattr(module, "on_session_ready", None) - if fn is None: - return None - if not inspect.iscoroutinefunction(fn): - return ValidationCheck( - name="on_session_ready_async", - passed=False, - message=( - "on_session_ready() must be async: found sync function. " - "Use 'async def on_session_ready(coordinator) -> None:'" - ), - severity="error", - ) - # B5 fix: validate arity — must accept at least one positional arg (coordinator) - try: - sig = inspect.signature(fn) - positional_params = [ - p - for p in sig.parameters.values() - if p.kind - in ( - inspect.Parameter.POSITIONAL_OR_KEYWORD, - inspect.Parameter.POSITIONAL_ONLY, - ) - and p.default is inspect.Parameter.empty - ] - if len(positional_params) < 1: - return ValidationCheck( - name="on_session_ready_async", - passed=False, - message=( - "on_session_ready() must accept a coordinator argument: " - "async def on_session_ready(coordinator) -> None" - ), - severity="error", - ) - except (ValueError, TypeError): - pass # Can't inspect — let it pass; runtime will catch it - return None diff --git a/python/amplifier_core/validation/tool.py b/python/amplifier_core/validation/tool.py index f8f1429..e4bfe02 100644 --- a/python/amplifier_core/validation/tool.py +++ b/python/amplifier_core/validation/tool.py @@ -15,7 +15,7 @@ from .base import ValidationCheck from .base import ValidationResult -from .structural import check_on_session_ready +from .base import check_on_session_ready def _implements_tool_interface(obj: Any) -> bool: diff --git a/scripts/e2e-smoke-test.sh b/scripts/e2e-smoke-test.sh index 5ed51da..f09c849 100755 --- a/scripts/e2e-smoke-test.sh +++ b/scripts/e2e-smoke-test.sh @@ -126,6 +126,49 @@ WHEEL=$(ls "$WHEEL_DIR"/amplifier_core-*.whl 2>/dev/null | head -1) [[ -z "$WHEEL" ]] && fail "No wheel found in $WHEEL_DIR/ — run without --skip-build first" log "Using wheel: $(basename "$WHEEL")" +# --------------------------------------------------------------------------- +# Step 1b: Pristine-import preflight +# --------------------------------------------------------------------------- +# Catch the v1.4.0 class of bug: wheel requires a runtime dep not declared in +# pyproject.toml, but masked by transitive deps in the polluted CLI install +# environment from Steps 4–5. We install ONLY the wheel into a fresh +# python:3.12-slim image and verify the production import paths succeed. +# +# Required because Step 4's `uv tool install git+microsoft/amplifier@main` +# pulls a full dep closure (including pytest as a transitive) which can hide +# missing runtime deps. A clean end-user `pip install amplifier-core` doesn't +# get that pollution and would fail. +# +# See context/release-mandate.md Incident History #5 (v1.4.0 yank). + +log "Pristine-import preflight: wheel must import on bare python:3.12-slim..." +WHEEL_BASENAME="$(basename "$WHEEL")" +docker run --rm \ + -v "$WHEEL":"/tmp/${WHEEL_BASENAME}":ro \ + -e WHEEL_BASENAME="$WHEEL_BASENAME" \ + python:3.12-slim \ + bash -c ' + set -e + pip install -q "/tmp/${WHEEL_BASENAME}" + python -c " +import sys +# Defensive: poison pytest so any leak is detected even on images that happen to ship it +sys.modules[\"pytest\"] = None +from amplifier_core.validation import ( + HookValidator, ToolValidator, OrchestratorValidator, + ProviderValidator, ContextValidator, +) +from amplifier_core.validation.base import check_on_session_ready +import amplifier_core._session_init # noqa: F401 +import amplifier_core.loader # noqa: F401 +import amplifier_core.coordinator # noqa: F401 +import amplifier_core.hooks # noqa: F401 +print(\"pristine import OK\") +" + ' || fail "Pristine-import preflight failed — wheel has runtime dep not declared in pyproject.toml" + +log "Pristine-import preflight passed." + # --------------------------------------------------------------------------- # Step 2: Create container # --------------------------------------------------------------------------- diff --git a/tests/test_pristine_validation_imports.py b/tests/test_pristine_validation_imports.py new file mode 100644 index 0000000..92503c3 --- /dev/null +++ b/tests/test_pristine_validation_imports.py @@ -0,0 +1,113 @@ +"""Regression for v1.4.0: validators must not require pytest at import time. + +The shipped v1.4.0 wheel had ``validation/structural/__init__.py`` eagerly +import test base classes (``test_context.py`` etc.) that did ``import pytest`` +at module top level. The 5 type validators imported ``check_on_session_ready`` +from ``.structural`` rather than ``.base``, so any production code path that +loaded a validator transitively pulled in ``pytest`` — which is not declared +as a runtime dependency, so a clean ``pip install amplifier-core`` failed +``amplifier`` startup with ``ModuleNotFoundError: No module named 'pytest'``. + +This test runs the production import path in a subprocess with ``pytest`` +poisoned (``sys.modules['pytest'] = None``) and asserts the imports succeed. +A subprocess is required because pytest is already imported into the parent +test process; only a fresh interpreter sees the poisoned state. + +If this test starts failing, it means somebody put a ``from .structural`` +back into a validator (or added a new pytest-dependent import to the +structural-package init), which would re-introduce the v1.4.0 regression. + +See ``context/release-mandate.md`` Incident History entry for v1.4.0. +""" + +import subprocess +import sys +import textwrap + + +def _run_in_pristine_subprocess(script: str) -> subprocess.CompletedProcess: + """Run a Python snippet in a subprocess with no pytest available.""" + return subprocess.run( + [sys.executable, "-c", script], + capture_output=True, + text=True, + timeout=30, + check=False, + ) + + +def test_validators_import_without_pytest() -> None: + """The 5 type validators must be importable without pytest installed.""" + script = textwrap.dedent(""" + import sys + sys.modules['pytest'] = None # poison: any `import pytest` now fails + + from amplifier_core.validation import ( + HookValidator, + ToolValidator, + OrchestratorValidator, + ProviderValidator, + ContextValidator, + ) + # Touch each so the import isn't optimised away. + for cls in (HookValidator, ToolValidator, OrchestratorValidator, + ProviderValidator, ContextValidator): + assert cls.__name__ + print("OK") + """) + result = _run_in_pristine_subprocess(script) + assert result.returncode == 0, ( + f"Validators failed to import without pytest.\n" + f"stdout={result.stdout!r}\n" + f"stderr={result.stderr!r}" + ) + assert "OK" in result.stdout + + +def test_check_on_session_ready_importable_without_pytest() -> None: + """``check_on_session_ready`` must be importable from base without pytest. + + The function lives in ``validation.base`` so per-type validators can call + it without dragging in the pytest-dependent test classes from + ``validation.structural``. + """ + script = textwrap.dedent(""" + import sys + sys.modules['pytest'] = None + + from amplifier_core.validation.base import check_on_session_ready + assert callable(check_on_session_ready) + print("OK") + """) + result = _run_in_pristine_subprocess(script) + assert result.returncode == 0, ( + f"check_on_session_ready not importable from base without pytest.\n" + f"stdout={result.stdout!r}\n" + f"stderr={result.stderr!r}" + ) + assert "OK" in result.stdout + + +def test_session_init_importable_without_pytest() -> None: + """``amplifier_core._session_init`` must be importable without pytest. + + This is the actual production code path that runs at every ``amplifier`` + startup. The v1.4.0 failure surfaced when ``initialize_session()`` + triggered loader → validators → structural → test_*.py → ``import pytest``. + """ + script = textwrap.dedent(""" + import sys + sys.modules['pytest'] = None + + import amplifier_core._session_init # noqa: F401 + import amplifier_core.loader # noqa: F401 + import amplifier_core.coordinator # noqa: F401 + print("OK") + """) + result = _run_in_pristine_subprocess(script) + assert result.returncode == 0, ( + f"Session-init import path failed without pytest.\n" + f"stdout={result.stdout!r}\n" + f"stderr={result.stderr!r}" + ) + assert "OK" in result.stdout diff --git a/uv.lock b/uv.lock index f2e0a73..34fc671 100644 --- a/uv.lock +++ b/uv.lock @@ -4,7 +4,7 @@ requires-python = ">=3.11" [[package]] name = "amplifier-core" -version = "1.3.2" +version = "1.4.1" source = { editable = "." } dependencies = [ { name = "click" },