diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 8916b8b..a953abb 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -23,9 +23,81 @@ jobs: - name: Run tests run: uv run --python ${{ matrix.python-version }} pytest --tb=short -q + build: + name: Build and verify artifacts + needs: test + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + - uses: astral-sh/setup-uv@v8.2.0 + # --no-sources proves the build works with `tool.uv.sources` + # disabled, which is how every other build tool — and therefore + # PyPI — sees this project. Cheap insurance today (we declare no + # sources), load-bearing the moment a path or git source is added. + - name: Build sdist and wheel + run: uv build --no-sources + - name: Verify artifact metadata and contents + run: | + uvx twine check dist/* + uvx check-wheel-contents dist/*.whl + # Everything downstream consumes THIS artifact. Rebuilding in the + # publish job would mean shipping bytes nothing ever installed. + - name: Upload dist + uses: actions/upload-artifact@v7 + with: + name: dist + path: dist/ + if-no-files-found: error + + install-matrix: + name: Install ${{ matrix.label }} (Python ${{ matrix.python-version }}) + needs: build + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + python-version: ["3.12", "3.13"] + include: + # The bare install is the important row: it is the only + # configuration where the vendor SDKs are genuinely absent, so + # it is the only place the lazy-import invariant can actually + # fail. `uv sync --all-extras` in the test job can never catch it. + - { extra: "", label: "no extras" } + - { extra: "[claude]", label: "claude" } + - { extra: "[copilot]", label: "copilot" } + - { extra: "[openai-compat]", label: "openai-compat" } + - { extra: "[bedrock]", label: "bedrock" } + - { extra: "[opencode]", label: "opencode" } + - { extra: "[all]", label: "all" } + steps: + - uses: actions/checkout@v7 + - uses: astral-sh/setup-uv@v8.2.0 + - uses: actions/download-artifact@v8 + with: + name: dist + path: dist/ + # Install the wheel by explicit PATH, not by name. `--find-links` + # only *adds* to the index, so if PyPI already carries a version + # newer than the one being released, the resolver picks that one and + # the whole matrix silently verifies the previously-published + # package instead of the artifact about to ship. Appending the extra + # to the path still resolves that extra's dependencies from PyPI, + # which is the behaviour we want. + - name: Install the built wheel into a clean environment + run: | + uv venv --python ${{ matrix.python-version }} + WHEEL=$(ls dist/*.whl) + echo "Installing ${WHEEL}${{ matrix.extra }}" + uv pip install "${WHEEL}${{ matrix.extra }}" + # Invoke the venv interpreter directly rather than through `uv run`, + # so there is no chance of the repo checkout's own project env being + # picked up instead of the environment the wheel was installed into. + - name: Smoke the installed distribution + run: .venv/bin/python scripts/smoke_install.py + release: name: Create GitHub Release - needs: test + needs: install-matrix runs-on: ubuntu-latest permissions: contents: write @@ -139,15 +211,24 @@ jobs: publish: name: Publish to PyPI - needs: [test, release] + needs: [install-matrix, release] runs-on: ubuntu-latest environment: pypi permissions: id-token: write steps: - - uses: actions/checkout@v7 - - uses: astral-sh/setup-uv@v8.2.0 - - name: Build package - run: uv build + # No checkout, no rebuild. Publishing the exact artifact the + # install matrix exercised is the whole point of this restructure: + # a rebuild here could differ from what was verified. + # + # download-artifact v8 enforces artifact hash checks by default — + # a mismatch against what `build` uploaded is an error, not a + # warning. That is the integrity guarantee this restructure needs, + # so no separate checksum step is required. + - name: Download the verified dist + uses: actions/download-artifact@v8 + with: + name: dist + path: dist/ - name: Publish to PyPI uses: pypa/gh-action-pypi-publish@release/v1 diff --git a/scripts/smoke_install.py b/scripts/smoke_install.py new file mode 100644 index 0000000..67b1a05 --- /dev/null +++ b/scripts/smoke_install.py @@ -0,0 +1,204 @@ +#!/usr/bin/env python3 +"""Smoke-test an *installed* ``airframe-agents`` distribution. + +Run against a clean environment that has the built wheel installed — not +against the source tree. The release workflow's ``install-matrix`` job runs +this once per (Python version x pip extra) combination against the exact +artifact it is about to publish. + +The point is to exercise things the in-repo test suite structurally cannot: + +* ``uv sync --all-extras --group dev`` installs the *source tree* with every + vendor SDK present. A wheel that ships the wrong files, declares a runtime + dependency that only the dev group was satisfying, or carries a broken + extra passes that gate and fails on a user's machine. +* The lazy-SDK-import invariant is only meaningfully testable where the + vendor SDKs are **absent**. In the ``no extras`` matrix row they are. + +Deliberately imports nothing beyond the standard library and ``airframe`` +itself — importing a vendor SDK here would defeat the check it performs. + +Usage:: + + .venv/bin/python scripts/smoke_install.py + +Exits non-zero on the first failed check, printing what broke. +""" + +from __future__ import annotations + +import importlib.metadata +import importlib.util +import subprocess +import sys + +#: Vendor SDKs that ``import airframe`` must never pull in. Adapter modules +#: import these inside the method that needs them, gated on the optional +#: extra being installed; a module-level import would make every consumer +#: pay for every vendor. +VENDOR_SDKS = ( + "claude_agent_sdk", + "anthropic", + "openai", + "aioboto3", + "botocore", + "opencode_ai", + "copilot", + "tiktoken", +) + +_failures: list[str] = [] + + +def check(name: str, condition: bool, detail: str = "") -> None: + """Record one check; print immediately so CI logs stay readable.""" + status = "ok " if condition else "FAIL" + print(f" [{status}] {name}" + (f" — {detail}" if detail else "")) + if not condition: + _failures.append(f"{name}: {detail}" if detail else name) + + +def main() -> int: + print("Smoke-testing the installed airframe-agents distribution\n") + print(f" python: {sys.version.split()[0]}") + print(f" exe: {sys.executable}\n") + + # --- The distribution is actually installed under its PyPI name ------- + try: + dist_version = importlib.metadata.version("airframe-agents") + except importlib.metadata.PackageNotFoundError: + print(" [FAIL] airframe-agents is not installed in this environment") + return 1 + print(f"Distribution: airframe-agents {dist_version}\n") + + # --- Which extras made it in? Reported, not asserted: the matrix row -- + # --- decides, and the row's own name already records the intent. ------ + installed_sdks = [m for m in VENDOR_SDKS if importlib.util.find_spec(m) is not None] + bare_install = not installed_sdks + print(f"Vendor SDKs present: {installed_sdks or '(none — bare install)'}\n") + + print("Checks:") + + # --- The import itself ------------------------------------------------ + try: + import airframe + except Exception as exc: # noqa: BLE001 - the whole point is to report it + check("import airframe", False, f"{type(exc).__name__}: {exc}") + return _summary() + + check("import airframe", True) + + # --- Lazy SDK imports ------------------------------------------------- + # Only decisive on the bare row, but harmless and still meaningful + # elsewhere: a module-level vendor import would show up here too. + leaked = sorted(m for m in VENDOR_SDKS if m in sys.modules) + check( + "import airframe pulls in no vendor SDK", + not leaked, + f"leaked {leaked}" if leaked else "", + ) + + # --- Version is single-sourced from metadata -------------------------- + check( + "airframe.__version__ matches distribution metadata", + getattr(airframe, "__version__", None) == dist_version, + f"__version__={getattr(airframe, '__version__', None)!r} vs {dist_version!r}", + ) + + # --- Public surface --------------------------------------------------- + missing = [name for name in getattr(airframe, "__all__", []) if not hasattr(airframe, name)] + check( + "every name in __all__ is importable", + not missing, + f"missing {missing}" if missing else f"{len(getattr(airframe, '__all__', []))} names", + ) + + # --- Discovery -------------------------------------------------------- + try: + declared = set(airframe.list_providers(installed_only=False)) + available = set(airframe.list_providers()) + except Exception as exc: # noqa: BLE001 + check("list_providers()", False, f"{type(exc).__name__}: {exc}") + return _summary() + + # Deliberately not asserting an exact roster — that belongs to + # tests/test_discovery.py, which runs on every PR and does not need a + # release to catch a regression. What only an installed artifact can + # prove is that discovery imports at all from the shipped files. + check( + "list_providers(installed_only=False) returns a non-empty roster", + bool(declared), + f"{len(declared)} providers: {sorted(declared)}", + ) + check( + "list_providers() is a subset of declared", + available <= declared, + f"available={sorted(available)}", + ) + + # The real packaging check, and the roster-independent one: every + # adapter module the wheel claims to serve must actually be importable + # from the wheel. A module dropped by a bad `packages =` or a stray + # sdist exclude shows up here as an unexpected exception rather than + # the documented ImportError-naming-an-extra. + for provider_id in sorted(declared): + try: + airframe.runtime_for(provider_id) + outcome = "class returned" + ok = True + except ImportError as exc: + # Documented: SDK absent for this extra. Fine, and the message + # is contractually required to name the extra to install. + ok = "airframe-agents[" in str(exc) + outcome = "ImportError names extra" if ok else str(exc)[:80] + except Exception as exc: # noqa: BLE001 + outcome = f"{type(exc).__name__}: {exc}"[:100] + ok = False + check(f"runtime_for({provider_id!r}) resolves from the wheel", ok, outcome) + + # A bare install has no vendor SDKs, so nothing is runnable. That is the + # honest signal the discovery layer documents — and a package that + # reports providers it cannot construct is worse than one reporting none. + if bare_install: + check( + "bare install reports no runnable providers", + available == set(), + f"reported {sorted(available)} with no SDKs installed", + ) + else: + check( + "installed extras surface at least one provider", + bool(available), + f"SDKs {installed_sdks} present but list_providers() is empty", + ) + + # --- Console script --------------------------------------------------- + # The entry point is declared in [project.scripts]; a wheel can ship the + # code and still get the script wiring wrong. + proc = subprocess.run( + [sys.executable, "-m", "airframe.cli", "providers"], + capture_output=True, + text=True, + ) + check( + "airframe.cli runs as a module", + proc.returncode == 0, + (proc.stderr or proc.stdout).strip()[:160], + ) + + return _summary() + + +def _summary() -> int: + print() + if _failures: + print(f"{len(_failures)} check(s) failed:") + for failure in _failures: + print(f" - {failure}") + return 1 + print("All checks passed.") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/test_lazy_imports.py b/tests/test_lazy_imports.py new file mode 100644 index 0000000..60a1423 --- /dev/null +++ b/tests/test_lazy_imports.py @@ -0,0 +1,149 @@ +"""``import airframe`` must not drag in any vendor SDK. + +CLAUDE.md states the invariant: adapter modules import their vendor SDK +*inside the method that needs it*, gated on the optional extra being +installed. A module-level import would make every consumer pay the import +cost of every vendor — and would break the bare +``pip install airframe-agents`` case outright, since the SDK would be +absent. + +These tests run in a subprocess with a clean interpreter, because +``sys.modules`` in the pytest process is already polluted: the rest of the +suite imports the vendor SDKs deliberately, and the dev environment +installs all of them. Checking ``sys.modules`` in-process would either +pass vacuously or fail for reasons unrelated to airframe. + +This is the PR-time counterpart to ``scripts/smoke_install.py``, which +performs the same check against a *built wheel* in an environment where +the SDKs are genuinely absent. Both are worth having: this one catches the +regression on the commit that introduces it; that one catches the case +where the packaging, rather than the code, is what went wrong. +""" + +from __future__ import annotations + +import subprocess +import sys + +import pytest + +#: Vendor SDKs no airframe import path may pull in eagerly. Keep in sync +#: with the same list in ``scripts/smoke_install.py``. +VENDOR_SDKS = ( + "claude_agent_sdk", + "anthropic", + "openai", + "aioboto3", + "botocore", + "opencode_ai", + "copilot", + "tiktoken", +) + + +def _imported_sdks_after(statement: str) -> list[str]: + """Return the vendor SDKs present in ``sys.modules`` after ``statement``. + + Runs in a fresh interpreter so the result reflects only what + ``statement`` caused to be imported. + + Args: + statement: Python source executed before the check. + + Returns: + Sorted vendor SDK module names found in ``sys.modules``. + + Raises: + AssertionError: The subprocess itself failed, which means the + statement could not even be executed. + """ + probe = ( + f"{statement}\n" + "import sys, json\n" + f"print(json.dumps(sorted(m for m in {VENDOR_SDKS!r} if m in sys.modules)))\n" + ) + proc = subprocess.run( + [sys.executable, "-c", probe], + capture_output=True, + text=True, + ) + assert proc.returncode == 0, ( + f"probe failed to execute {statement!r}:\n{proc.stderr or proc.stdout}" + ) + import json + + return list(json.loads(proc.stdout.strip().splitlines()[-1])) + + +def test_import_airframe_pulls_in_no_vendor_sdk() -> None: + """The headline invariant — plain ``import airframe`` is cheap.""" + leaked = _imported_sdks_after("import airframe") + assert not leaked, ( + f"`import airframe` eagerly imported {leaked}. Adapter modules must " + f"import their vendor SDK inside the method that needs it, so a " + f"consumer who installed no extras can still `import airframe`." + ) + + +def test_discovery_pulls_in_no_vendor_sdk() -> None: + """``list_providers()`` inspects adapters without importing their SDKs. + + Discovery decides availability with ``importlib.util.find_spec`` on + each adapter's ``REQUIRES_PACKAGE``. ``find_spec`` locates a module + without executing it; switching to a real import would make the + menu-building path as expensive as using every adapter at once. + """ + leaked = _imported_sdks_after( + "import airframe\nairframe.list_providers()\nairframe.list_providers(installed_only=False)" + ) + assert not leaked, f"discovery eagerly imported {leaked}" + + +@pytest.mark.parametrize( + "module", + [ + "airframe.adapters.claude_code", + "airframe.adapters.copilot", + "airframe.adapters.bedrock", + "airframe.adapters.opencode_server", + "airframe.adapters.opencode_zen", + "airframe.adapters.opencode_go", + "airframe.adapters.openrouter", + "airframe.adapters.openai_compatible", + ], +) +def test_adapter_module_import_pulls_in_no_vendor_sdk(module: str) -> None: + """Importing an adapter *module* is still free. + + Stronger than the top-level check and the one that actually regresses: + a contributor adding ``import openai`` at the top of an adapter breaks + this while `import airframe` might still look fine if that adapter is + not re-exported. + """ + leaked = _imported_sdks_after(f"import {module}") + assert not leaked, ( + f"`import {module}` eagerly imported {leaked}. Move the vendor " + f"import inside the method that needs it." + ) + + +def test_vendor_sdk_list_covers_every_declared_requirement() -> None: + """Guard the guard: every adapter's ``REQUIRES_PACKAGE`` is watched. + + Without this, adding an adapter with a new vendor SDK would silently + create an unwatched eager-import path — the test above would keep + passing while the invariant quietly stopped being enforced. + """ + from airframe.discovery import _builtin_runtime_classes + + required = { + pkg + for cls in _builtin_runtime_classes() + if (pkg := getattr(cls, "REQUIRES_PACKAGE", None)) + } + unwatched = sorted(required - set(VENDOR_SDKS)) + assert not unwatched, ( + f"{unwatched} is declared as a REQUIRES_PACKAGE but absent from " + f"VENDOR_SDKS — add it here and in scripts/smoke_install.py so the " + f"lazy-import invariant is actually enforced for it." + )