From 0e4a9fd02fff775046c4caaf0549d5c1b610be8f Mon Sep 17 00:00:00 2001 From: Paul O'Fallon Date: Sun, 23 Aug 2026 23:46:03 +0000 Subject: [PATCH 1/2] ci: verify the built artifact before publishing it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The release workflow's `test` job runs `uv sync --all-extras --group dev`, which installs the *source tree*. `publish` then ran `uv build` and shipped the result. Nothing between them ever installed the wheel, so the first person to install a release was a user, and every packaging-layer bug reached PyPI unopposed: a wrong `packages =`, a runtime dependency only the dev group was satisfying, a broken extra, an sdist exclude that dropped something needed at runtime. Restructures to test → build → install-matrix → release → publish: - `build` runs `uv build --no-sources`, then `twine check` and `check-wheel-contents`, and uploads dist/ as an artifact. `--no-sources` proves the build works with `tool.uv.sources` disabled, which is how every other build tool sees the project. No sources are declared today, so it is insurance rather than a fix. - `install-matrix` installs that artifact into a clean venv across {3.12, 3.13} x {no extras, each of the six extras, all} and runs scripts/smoke_install.py against it. - `publish` no longer checks out or rebuilds. It downloads and ships the exact artifact the matrix exercised; a rebuild there could differ from what was verified. The bare "no extras" row is the point. It is the only configuration where the vendor SDKs are genuinely absent, so it is the only place the lazy-import invariant from CLAUDE.md can actually fail — and it is a configuration CI has never once executed. Install by wheel PATH, not by name. This was found the hard way while testing locally: `--find-links` only *adds* to the index, so with PyPI already carrying 0.9.2 and the local build at 0.9.1, the resolver picked 0.9.2 and the smoke test silently verified the previously-published package. Appending the extra to the path still resolves that extra's dependencies from PyPI, which is the behaviour wanted. Also adds tests/test_lazy_imports.py so the invariant is enforced at PR time rather than only at release. It probes in a subprocess because the pytest process has already imported the vendor SDKs deliberately, which would make an in-process sys.modules check pass vacuously. Verified it fails: injecting a top-level `import openai` into openrouter.py turns 10 of the 11 tests red. The last test guards the guard — it asserts every adapter's REQUIRES_PACKAGE appears in the watched list, so a new adapter with a new SDK cannot silently create an unwatched eager-import path. scripts/smoke_install.py deliberately does not assert an exact provider roster; tests/test_discovery.py already does, on every PR. What only an installed artifact can prove is that every adapter module the wheel claims to serve is importable from the wheel, which it checks via runtime_for() per provider. Verified end to end locally against a real build: all seven extras rows pass 15/15 checks, twine check and check-wheel-contents both clean. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UBTr6Q6kTGUMQiwwBHcdMj --- .github/workflows/release.yml | 88 ++++++++++++++- scripts/smoke_install.py | 204 ++++++++++++++++++++++++++++++++++ tests/test_lazy_imports.py | 149 +++++++++++++++++++++++++ 3 files changed, 435 insertions(+), 6 deletions(-) create mode 100644 scripts/smoke_install.py create mode 100644 tests/test_lazy_imports.py diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 8916b8b..bcb7a27 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@v4 + 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@v4 + 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,19 @@ 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. + - name: Download the verified dist + uses: actions/download-artifact@v4 + 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." + ) From 136185636693764139bd96197ff72e7b01c0c5d1 Mon Sep 17 00:00:00 2001 From: Paul O'Fallon Date: Mon, 24 Aug 2026 01:20:38 +0000 Subject: [PATCH 2/2] ci: bump artifact actions to current majors (v4 -> v7/v8) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit v4 is several majors stale. Checked the release notes across v5-v8: the breaking changes are Node runtime bumps and download path/decompression behaviour, not input removals — name, path, retention-days and if-no-files-found are unchanged. download-artifact v8 also enforces artifact hash checks by default rather than warning on mismatch, which supplies the build-to-publish integrity guarantee this restructure depends on. Noted inline so the absence of a separate checksum step reads as deliberate. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UBTr6Q6kTGUMQiwwBHcdMj --- .github/workflows/release.yml | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index bcb7a27..a953abb 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -43,7 +43,7 @@ jobs: # Everything downstream consumes THIS artifact. Rebuilding in the # publish job would mean shipping bytes nothing ever installed. - name: Upload dist - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v7 with: name: dist path: dist/ @@ -72,7 +72,7 @@ jobs: steps: - uses: actions/checkout@v7 - uses: astral-sh/setup-uv@v8.2.0 - - uses: actions/download-artifact@v4 + - uses: actions/download-artifact@v8 with: name: dist path: dist/ @@ -220,8 +220,13 @@ jobs: # 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@v4 + uses: actions/download-artifact@v8 with: name: dist path: dist/