Skip to content

fix(v1.4.1): pristine validation imports + smoke-test preflight - #67

Merged
Brian Krabach (bkrabach) merged 1 commit into
mainfrom
fix/v1.4.1-pristine-validation-imports
Apr 25, 2026
Merged

fix(v1.4.1): pristine validation imports + smoke-test preflight#67
Brian Krabach (bkrabach) merged 1 commit into
mainfrom
fix/v1.4.1-pristine-validation-imports

Conversation

@bkrabach

Copy link
Copy Markdown
Collaborator

Incident response — v1.4.0 broken on PyPI

Do not merge this PR yourself. When CI is green, ping me — I (the core owner) will do the merge + tag push under the new pre-merge gate so the merge button click is the release click. v1.4.0 will be yanked from PyPI after v1.4.1 publishes (yanking before would expose users in the gap window to v1.3.3 fallback).

What broke in v1.4.0

A clean pip install amplifier-core==1.4.0 followed by amplifier startup raises:

Error: Session initialization failed: RuntimeError: Cannot initialize without orchestrator: No module named 'pytest'

Reproduced in pristine docker python:3.12-slim. Import chain:

amplifier_core/validation/__init__.py:39 → from .context import ContextValidator
amplifier_core/validation/context.py:18  → from .structural import check_on_session_ready
amplifier_core/validation/structural/__init__.py:37 → from .test_context import ContextStructuralTests
amplifier_core/validation/structural/test_context.py:14 → import pytest    ❌ ImportError

pytest is not declared as a runtime dep (it's a dev/test dep). The 5 type validators imported check_on_session_ready from .structural, dragging in the test base classes that import pytest at module top level. The author's PR #63 commit message claimed check_on_session_ready lived in validation/base.py — it didn't.

Why the smoke test missed it

e2e-smoke-test.sh Step 4 (uv tool install git+microsoft/amplifier@main) pulls pytest as a transitive dep into the test venv. That venv pollution masked the missing runtime declaration — the smoke test environment was never actually pristine. A clean end-user pip install amplifier-core doesn't get that pollution and fails.

What this PR does

Fix the import (production code)

File Change
validation/base.py Added check_on_session_ready() function + import inspect + Any
validation/structural/__init__.py Removed function definition; kept re-export from ..base import check_on_session_ready for backward compat
validation/{hook,tool,orchestrator,provider,context}.py 5 files: from .structuralfrom .base

Add a regression test

tests/test_pristine_validation_imports.py — 3 tests using subprocess + sys.modules['pytest'] = None poisoning to verify validators, check_on_session_ready from base, and _session_init import path all work without pytest. Runs in <1s in any Python CI without needing docker. Would have caught v1.4.0 deterministically.

Tighten the smoke test (gate fix)

scripts/e2e-smoke-test.sh — new Step 1b: pristine-import preflight that mounts the freshly built wheel into a bare python:3.12-slim and imports the production code paths. Runs before any polluted CLI install. Catches the v1.4.0 class of bug in ~10s.

Bump version

pyproject.toml, crates/amplifier-core/Cargo.toml, bindings/python/Cargo.toml1.4.1 (atomic via scripts/bump_version.py).

Document the incident

Added row #5 to the Incident History table in context/release-mandate.md.

Verification

Targeted tests:

tests/test_pristine_validation_imports.py     3 passed
tests/test_check_on_session_ready.py         18 passed
tests/test_session_init_on_session_ready.py   3 passed
tests/test_loader_on_session_ready.py         8 passed (existing)
                                             ─────────
                                             34 passed, 0 regressions

Pristine-import sanity (without docker):

$ python3 -c "
import sys
sys.modules['pytest'] = None
from amplifier_core.validation import HookValidator, ToolValidator, ...
print('FIX VERIFIED')
"
FIX VERIFIED: production import path works without pytest

Full E2E smoke test (SMOKE_TIMEOUT=360 ./scripts/e2e-smoke-test.sh):

[smoke-test] Pristine-import preflight: wheel must import on bare python:3.12-slim...
[smoke-test] Pristine-import preflight passed.
... (full session with recipe execution) ...
[PASS] ========================================================
[PASS]  SMOKE TEST PASSED
[PASS]  amplifier, version 2026.04.23-b7e0da3 (core 1.4.1)
[PASS]  No crashes, no tool failures, no timeout
[PASS] ========================================================

Pre-merge gate compliance (per context/release-mandate.md)

  • Version bump in PR (1.4.1) ✅
  • Rust/Python symmetry — N/A (no new constants in this PR) ✅
  • Freshly built wheel — used in smoke test ✅
  • E2E smoke test result posted (above) ✅
  • No [tool.uv.sources] git overrides ✅
  • Core owner merges — please do not merge this; ping me when CI is green

Follow-ups (separate tickets, not blocking)

  1. Audit which package in git+microsoft/amplifier@main's dep closure declares pytest as a non-optional dep — that misdeclaration was the root upstream cause of the smoke-test gap.
  2. v1.4.2: add PEP 562 __getattr__ lazy loading in validation/structural/__init__.py so the package can be imported without pulling in test classes by default — defense-in-depth so future contributors can't reintroduce this class of bug by adding another non-test helper next to check_on_session_ready.

v1.4.0 shipped to PyPI broken: clean `pip install amplifier-core==1.4.0`
followed by `amplifier` startup raised `ModuleNotFoundError: No module
named 'pytest'` because validators transitively pulled pytest as a runtime
dep through `validation/structural/__init__.py`. The 5 type validators
imported `check_on_session_ready` from `.structural` instead of `.base`,
and `structural/__init__.py` eagerly loads test base classes that
`import pytest` at module top level.

Fix:
- Move `check_on_session_ready()` from `validation/structural/__init__.py`
  to `validation/base.py` (function only depends on stdlib + `ValidationCheck`)
- Update 5 validator files to import from `.base` instead of `.structural`
- Keep re-export in `validation/structural/__init__.py` for backward compat

Smoke-test gap fix:
- The v1.4.0 e2e-smoke-test.sh missed this because Step 4 `uv tool install
  git+microsoft/amplifier@main` pulls pytest as a transitive into the venv,
  masking the missing runtime declaration. Added Step 1b: pristine-import
  preflight that installs ONLY the wheel into a bare python:3.12-slim and
  imports the production code paths. Catches this class of bug in <30s
  before any polluted CLI install runs.

Regression test:
- Added `tests/test_pristine_validation_imports.py` (3 tests, subprocess +
  `sys.modules['pytest'] = None` poisoning). Runs in any Python CI without
  needing docker; would have caught v1.4.0 deterministically.

Documentation:
- Added incident #5 (v1.4.0) row to `context/release-mandate.md` Incident
  History.

Verification:
- 34/34 targeted tests pass
- Full e2e-smoke-test.sh passed end-to-end with new preflight step
- pristine-import sanity confirmed locally with `sys.modules['pytest'] = None`

🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
@bkrabach
Brian Krabach (bkrabach) merged commit 12f3d38 into main Apr 25, 2026
6 checks passed
@bkrabach
Brian Krabach (bkrabach) deleted the fix/v1.4.1-pristine-validation-imports branch April 25, 2026 10:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant