From 80576bae06d9c27e0e0fd05394536b6f11e4c2c0 Mon Sep 17 00:00:00 2001 From: Jake Fineman Date: Fri, 4 Sep 2026 22:52:43 -0400 Subject: [PATCH 1/2] fix(ga): pre-publish wheel-import guard for sdk-python + repair stale isolation marker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All 7 checks the registry clean-room gate (run 33933317710) reported failing were already root-cause fixed in source before this change, across three repositories (verified independently in this lane, not merely re-read from docs): - @wave-av/cli npm-provenance-attested / bin-version-matches-package / declared-dep-ranges-pinned: fixed on wave-av/cli's origin/main (release.yml already runs `npm publish --provenance` under `id-token: write` OIDC; src/lib/version.ts derives CLI_VERSION from package.json at runtime; @wave-av/sdk pinned to exact "2.0.14"). Owned by a different repository; nothing to change here. - wave-sdk (PyPI) py-import-module / py-no-stdlib-shadow: fixed on wave-av/sdk-python's origin/main (module renamed wave -> wave_sdk, version 2.1.0, with its own tests/test_packaging.py + smoke-install.yml). Owned by a different repository; nothing to change here. - wave-av-sdk (PyPI) py-import-module / py-no-stdlib-shadow: fixed on THIS repo's origin/main already (sdk-python/ renamed wave/ -> wave_sdk/, version 3.0.0). Re-verified here by actually building the wheel, installing it into a fresh venv, and running scripts/ga/cleanroom_python_assert.py against the installed artifact — py-import-module and py-no-stdlib-shadow both pass; full pytest suite 31/31 (36/36 with the new tests below). None of the above needed a source change; all are blocked solely on an operator publish, which this lane may not perform. What this commit actually changes: 1. scripts/ga/cleanroom_python_assert.py: the cleanroom-isolation guard keyed its repo-checkout-on-sys.path detection on a hardcoded "wave" directory name. That was the PRE-rename package directory; after wave -> wave_sdk landed (in the same source fix this file is supposed to help verify), the guard silently stopped matching either checkout's real layout and could never again detect a genuine repo-on-path leak. Demonstrated the regression directly: with PYTHONPATH pointed at the repo root, the old code reports cleanroom-isolation ok=true (wrong); the fix (keyed on `args.module`, the same name already used for the import check) reports ok=false (correct). Re-ran the full probe against a real built wheel afterward to confirm it still passes cleanroom-isolation / py-import-module / py-no-stdlib-shadow. 2. sdk-python/tests/test_packaging.py (new): offline packaging guards — no shipped top-level package may shadow a stdlib name, `import wave` (bare) still resolves to the stdlib from inside the checkout, `wave_sdk.__version__` matches pyproject.toml's version, and the distribution name is still `wave-av-sdk`. These run in the normal `pytest` pass, before any wheel is ever built — closing the gap that let 2.0.0 ship broken in the first place (an editable install / repo-checkout test run hides the exact stdlib-shadow class this guards). Mirrors the equivalent guard already proven out in wave-av/sdk-python's tests/test_packaging.py, adapted to this package's name/module. pyproject.toml gains the matching `tomli` dev-extra for Python < 3.11 (tomllib is stdlib only from 3.11). 3. .github/workflows/test-python.yml: new `smoke-install` job — builds the real wheel, installs it (no `-e`, no repo on sys.path) into a throwaway venv, and runs THIS repo's own scripts/ga/registry-cleanroom.mjs probe (cleanroom_python_assert.py) against the installed artifact on every PR touching sdk-python. This is the same probe the GA gate runs against the live PyPI package after a publish — the difference is this one runs pre-publish, offline, on every PR, so the wave/wave_sdk defect class cannot reach a registry a second time. Verified end-to-end locally: built the wheel, installed into a fresh venv, ran the exact copy/probe/parse sequence the job runs — all three checks (cleanroom-isolation, py-import-module, py-no-stdlib-shadow) pass; actionlint clean on the workflow file. Verified: `pytest -q` sdk-python 36/36 pass (31 pre-existing + 5 new); `ruff check tests/test_packaging.py` clean; `actionlint .github/workflows/test-python.yml` clean; manual wheel build + fresh-venv install + cleanroom_python_assert.py probe pass against the real built artifact, both before and after the isolation-marker fix (with the fix demonstrated to catch a real injected leak the old code missed). Tracking: claude-workstation#4321 item 6. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01MLCfz2w3xiGLfFFgFmbe5j --- .github/workflows/test-python.yml | 47 ++++++++ scripts/ga/cleanroom_python_assert.py | 8 +- sdk-python/pyproject.toml | 3 + sdk-python/tests/test_packaging.py | 149 ++++++++++++++++++++++++++ 4 files changed, 206 insertions(+), 1 deletion(-) create mode 100644 sdk-python/tests/test_packaging.py diff --git a/.github/workflows/test-python.yml b/.github/workflows/test-python.yml index 59e0951..3a7cec2 100644 --- a/.github/workflows/test-python.yml +++ b/.github/workflows/test-python.yml @@ -7,11 +7,13 @@ on: pull_request: paths: - "sdk-python/**" + - "scripts/ga/cleanroom_python_assert.py" - ".github/workflows/test-python.yml" push: branches: [main] paths: - "sdk-python/**" + - "scripts/ga/cleanroom_python_assert.py" concurrency: group: ${{ github.workflow }}-${{ github.ref }} @@ -37,3 +39,48 @@ jobs: run: python -m pip install --upgrade pip && python -m pip install -e '.[dev]' - name: Test (offline) run: python -m pytest -q + + smoke-install: + # Regression guard for the fresh-install class of bug: builds the actual wheel from + # this checkout, installs it (no `-e`, no repo on sys.path) into a throwaway venv, + # and runs the SAME probe `scripts/ga/registry-cleanroom.mjs` runs against the live + # PyPI artifact after a publish. `python -m pytest` from the repo checkout does NOT + # catch this class of bug (the checkout dir is first on sys.path, which is exactly + # how wave-av-sdk 2.0.0's `wave` -> stdlib collision hid through review and CI) — this + # job is the one gate that runs it the way a real `pip install wave-av-sdk` user does, + # and it runs on every PR, before any registry publish is even possible. + runs-on: ubuntu-latest + strategy: + matrix: + python-version: ["3.10", "3.12"] + steps: + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 + - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5 + with: + python-version: ${{ matrix.python-version }} + - name: Build wheel + working-directory: sdk-python + run: python -m pip install --upgrade pip && pip install build && python -m build --wheel + - name: Create fresh venv (no repo on sys.path) + run: python -m venv "$RUNNER_TEMP/smoke" + - name: Install the built wheel + run: | + WHEEL=$(ls sdk-python/dist/*.whl) + "$RUNNER_TEMP/smoke/bin/pip" install --upgrade pip + "$RUNNER_TEMP/smoke/bin/pip" install "$WHEEL" + - name: Copy the clean-room probe into the smoke venv + run: cp scripts/ga/cleanroom_python_assert.py "$RUNNER_TEMP/smoke/cleanroom_python_assert.py" + - name: Assert the installed wheel imports and does not shadow the stdlib + working-directory: ${{ runner.temp }}/smoke + run: | + set -euo pipefail + out=$(bin/python cleanroom_python_assert.py --dist wave-av-sdk --module wave_sdk --symbol Wave) + echo "$out" + echo "$out" | bin/python -c ' + import json, sys + report = json.load(sys.stdin) + failed = [c["name"] for c in report["checks"] if not c["ok"]] + if failed: + print("FAILED:", failed) + sys.exit(1) + ' diff --git a/scripts/ga/cleanroom_python_assert.py b/scripts/ga/cleanroom_python_assert.py index de0345b..1da61f9 100644 --- a/scripts/ga/cleanroom_python_assert.py +++ b/scripts/ga/cleanroom_python_assert.py @@ -84,9 +84,15 @@ def main() -> int: checks: list[dict] = [] # Guard: a repo checkout on sys.path would make this whole run meaningless. + # Keyed on `args.module` (the same name the import check below uses), not a literal + # "wave" — that literal was the pre-rename package directory name, and after the + # `wave` -> `wave_sdk` rename (ART-001, this repo's sdk-python and the sibling + # wave-av/sdk-python repo both moved) a hardcoded "wave" here silently stopped + # matching either checkout's real layout, leaving this guard permanently blind + # to the exact repo-on-sys.path leak it exists to catch. repo_marker_on_path = [ p for p in sys.path - if p and os.path.isdir(os.path.join(p, "sdk-python", "wave")) + if p and os.path.isdir(os.path.join(p, "sdk-python", args.module)) ] checks.append(check( "cleanroom-isolation", diff --git a/sdk-python/pyproject.toml b/sdk-python/pyproject.toml index b81f9eb..ec3aad1 100644 --- a/sdk-python/pyproject.toml +++ b/sdk-python/pyproject.toml @@ -71,6 +71,9 @@ dev = [ "mypy>=1.0.0", "ruff>=0.1.0", "black>=23.0.0", + # tests/test_packaging.py reads this file back to assert the shipped metadata + # matches the repo (name/version). tomllib is stdlib from 3.11 only. + "tomli>=2.0.0; python_version < '3.11'", ] [project.urls] diff --git a/sdk-python/tests/test_packaging.py b/sdk-python/tests/test_packaging.py new file mode 100644 index 0000000..17bea32 --- /dev/null +++ b/sdk-python/tests/test_packaging.py @@ -0,0 +1,149 @@ +""" +Packaging / distribution-metadata guards. + +These tests exist because of a class of defect that NO other gate in this repo caught +offline, and that only became visible once the package was on PyPI — where it is +unfixable, since PyPI refuses a re-upload of an already-published version. + +Every published `wave-av-sdk` release through `2.0.1` shipped a top-level package named +`wave`. CPython's standard library ships `Lib/wave.py` (WAV audio I/O), and the stdlib +directory sits AHEAD of `site-packages` on `sys.path`. So `import wave` in a fresh +`pip install wave-av-sdk` resolved to the stdlib module and the entire SDK was +unreachable — the artifact was 100% unimportable via its own documented entry point, on +every Python version (`scripts/ga/registry-cleanroom.mjs` in this repo proved this +against the live PyPI artifact: `py-import-module` / `py-no-stdlib-shadow`, both FAIL on +`wave-av-sdk@2.0.0`). The repo checkout hid it during development: the checkout +directory is first on `sys.path`, so the local `wave/` package won `import wave` under +pytest, and `pip install -e .` (an editable install, which also just points back at the +checkout) hid it in CI too — neither ever exercised a real built-and-installed wheel. + +`.github/workflows/test-python.yml` (the `smoke-install` job) guards the same class at +the wheel level: build -> fresh venv -> install -> import using this repo's own +`scripts/ga/cleanroom_python_assert.py`, the exact probe the GA gate runs against the +live registry. These tests are the cheap, always-on half: they fail at PR time, in the +normal unit run, before a wheel is ever built. + +Guarded here: + 1. No top-level package this repo ships may shadow a stdlib module name. + 2. `import wave` (bare) must still resolve to the stdlib, from inside the checkout. + 3. `wave_sdk.__version__` must equal `[project] version` in pyproject.toml. + 4. `[project] name` must still be the distribution name README/CHANGELOG promise. +""" + +from __future__ import annotations + +import sys +import sysconfig +from pathlib import Path + +try: # tomllib is stdlib from 3.11; tomli is the dev-extra fallback below that. + import tomllib +except ModuleNotFoundError: # pragma: no cover - exercised on 3.10 only + import tomli as tomllib + +REPO_ROOT = Path(__file__).resolve().parent.parent +PYPROJECT = REPO_ROOT / "pyproject.toml" + + +def _pyproject() -> dict: + with PYPROJECT.open("rb") as fh: + return tomllib.load(fh) + + +def _stdlib_top_level_names() -> set[str]: + """Every name `import ` could resolve to from the standard library. + + `sys.stdlib_module_names` is 3.10+ (this package's floor), so that alone would + suffice, but the on-disk scan is unioned in too: it costs nothing and it means this + guard does not silently get weaker if it is ever backported below 3.10. + """ + names = set(sys.builtin_module_names) + names |= set(getattr(sys, "stdlib_module_names", ())) + stdlib_dir = Path(sysconfig.get_paths()["stdlib"]) + if stdlib_dir.is_dir(): + for entry in stdlib_dir.iterdir(): + if entry.suffix == ".py": + names.add(entry.stem) + elif entry.is_dir() and (entry / "__init__.py").exists(): + names.add(entry.name) + return names + + +def _shipped_top_level_packages() -> list[str]: + """Top-level importable packages in the checkout that setuptools will ship. + + Derived from the filesystem (any root-level directory with an `__init__.py`) rather + than from the pyproject include-glob (`wave_sdk*`), because the failure mode being + guarded is exactly someone re-adding a directory the glob would sweep up — reading + the glob back would make this test agree with the very config that could regress. + `tests`, `scripts` and `examples` are excluded: none is in + `[tool.setuptools.packages.find] include`, so none is ever part of the distribution. + """ + skip = {"tests", "scripts", "examples"} + return sorted( + p.name + for p in REPO_ROOT.iterdir() + if p.is_dir() + and not p.name.startswith((".", "_")) + and p.name not in skip + and (p / "__init__.py").exists() + ) + + +def test_repo_ships_the_wave_sdk_package(): + """Control for the shadow test below: prove the scan sees anything at all. + + Without this, a bug that made `_shipped_top_level_packages()` return `[]` would turn + the shadow guard into a test that can never fail. + """ + assert "wave_sdk" in _shipped_top_level_packages() + + +def test_no_shipped_package_shadows_a_stdlib_module(): + """A distribution package named after a stdlib module is permanently unimportable. + + site-packages comes AFTER the stdlib on sys.path, so the stdlib always wins. + """ + stdlib = _stdlib_top_level_names() + collisions = [name for name in _shipped_top_level_packages() if name in stdlib] + assert collisions == [], ( + f"top-level package(s) {collisions} collide with a Python standard-library " + f"module name. The stdlib precedes site-packages on sys.path, so a user who " + f"runs `pip install wave-av-sdk` could never import them. Rename the package " + f"(this is exactly the defect that shipped as wave-av-sdk 2.0.0's `wave`)." + ) + + +def test_import_wave_still_resolves_to_the_standard_library(): + """The specific regression: re-adding a top-level `wave/` here would break users. + + Run from the repo checkout, the checkout is first on sys.path — so if a `wave/` + package reappears, this assertion fails HERE, which is the one place the old bug + was invisible (both under pytest and under `pip install -e .`). + """ + import wave # noqa: F401 - imported for its resolved location, not its API + + stdlib_dir = Path(sysconfig.get_paths()["stdlib"]).resolve() + resolved = Path(wave.__file__).resolve() + assert stdlib_dir in resolved.parents, ( + f"`import wave` resolved to {resolved}, not the standard library at " + f"{stdlib_dir}. A top-level `wave` package has been reintroduced." + ) + assert REPO_ROOT not in resolved.parents, f"`import wave` resolved into this repo: {resolved}" + + +def test_dunder_version_matches_pyproject_version(): + """`wave_sdk.__version__` is what users print; pyproject is what PyPI records. + + A hardcoded literal in a test (or in `__init__.py` itself) can drift from + `pyproject.toml` with no build-time signal — this reads pyproject back rather than + encoding the expected value, so it fails the moment the two disagree either way. + """ + import wave_sdk + + assert wave_sdk.__version__ == _pyproject()["project"]["version"] + + +def test_distribution_name_is_wave_av_sdk(): + """`pip install wave-av-sdk` is what README/CHANGELOG currently promise.""" + assert _pyproject()["project"]["name"] == "wave-av-sdk" From b419a85054a85f3dd5563cf903897b3f18c7e7d4 Mon Sep 17 00:00:00 2001 From: Jake Fineman Date: Fri, 4 Sep 2026 23:44:27 -0400 Subject: [PATCH 2/2] fix(ga): make the clean-room isolation guard layout-independent and testable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `cleanroom-isolation` check is the load-bearing assertion of the registry clean-room gate: `py-import-module` and `py-no-stdlib-shadow` only mean anything if no source checkout can satisfy the import the PUBLISHED wheel is supposed to satisfy. It was keyed on one hardcoded path. The first fix on this branch replaced the stale `/sdk-python/wave` literal with `/sdk-python/`. That restores the guard for wave-av/sdks' own layout only. The probe also runs against wave-av/sdk-python, whose package sits at the REPOSITORY ROOT (`wave_sdk/`, verified on that repo's origin/main) — a checkout of it on sys.path still passed the guard. Measured, both directions: fake root-level checkout on PYTHONPATH, probe at this branch's head cleanroom-isolation ok=true (wrong — leak invisible) same sys.path, probe after this commit cleanroom-isolation ok=false REPO ON sys.path: [.../wave_sdk] `checkout_paths_providing()` now asks the layout-independent question — can this sys.path entry supply the module under test, at the entry itself or under any of the conventional source roots — after excluding the entries that legitimately can (site-packages, stdlib, user-site, the venv prefix), read from sysconfig and `site` rather than assumed to sit under sys.prefix. It also resolves the `''` entry to cwd, which the previous code skipped outright. cleanroom-targets.mjs now COPIES the probe into the throwaway room before running it. Python unconditionally prepends the executed script's directory to sys.path, so running it in place put `/scripts/ga` first on the path of every clean-room probe — a repository directory inside the clean room, which is the one thing this suite promises never happens. (`-P`/PYTHONSAFEPATH is 3.11+; the venv's interpreter version is not ours to assume.) test-python.yml's smoke-install job already copies it for exactly this reason. sdk-python/tests/test_cleanroom_probe.py: 8 offline unit tests over real directory trees — both repository layouts, single-file module, the cwd entry, the site-packages false-positive control, and the self-maintenance property (rename the package and the guard follows it). A hardcoded-path guard had no test that could catch it going blind; that is why the rename disarmed it silently. Verification in this lane: - pytest -q in sdk-python: 44/44 pass (36 before this commit + 8 new) - ruff check under the repo's own config: clean on both changed/added files - full pypi clean-room gate re-run against the LIVE registries with these changes: identical evidence fingerprint 8e6537fd84b4859d003e6787d3c4b585ceb5 ee39da5e681d4d4f2cb098cc8e04 to the run at this branch's head and to origin/main — the hardening changes no verdict on real artifacts Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01MLCfz2w3xiGLfFFgFmbe5j --- scripts/ga/cleanroom-targets.mjs | 13 +- scripts/ga/cleanroom_python_assert.py | 80 +++++++++++-- sdk-python/tests/test_cleanroom_probe.py | 144 +++++++++++++++++++++++ 3 files changed, 225 insertions(+), 12 deletions(-) create mode 100644 sdk-python/tests/test_cleanroom_probe.py diff --git a/scripts/ga/cleanroom-targets.mjs b/scripts/ga/cleanroom-targets.mjs index 42b0698..1e266aa 100644 --- a/scripts/ga/cleanroom-targets.mjs +++ b/scripts/ga/cleanroom-targets.mjs @@ -2,7 +2,7 @@ // hand a context to the checks. Nothing here reads the repository checkout. import { createHash } from 'node:crypto'; -import { mkdtempSync, writeFileSync } from 'node:fs'; +import { copyFileSync, mkdtempSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { dirname, join } from 'node:path'; import { fileURLToPath } from 'node:url'; @@ -131,7 +131,16 @@ export async function runPypiTarget(target, args) { // cwd is the throwaway room, never the repo: a checkout on sys.path could satisfy an import the // published wheel is supposed to satisfy — exactly the illusion this suite exists to destroy. // cleanroom_python_assert.py re-verifies that independently and reports it as its own check. - const argv = [join(HERE, 'cleanroom_python_assert.py'), '--dist', name, '--module', target.import_module]; + // + // The probe is COPIED into the room before it runs. Python unconditionally prepends the + // executed script's own directory to sys.path, so running it in place put `/scripts/ga` + // — a repository directory — first on the path of every clean-room probe, which is the one + // thing this whole suite promises never happens. (`-P`/PYTHONSAFEPATH would also fix it but + // is 3.11+; the venv's interpreter version is not ours to assume.) `.github/workflows/ + // test-python.yml`'s smoke-install job already copies the probe for the same reason. + const probePath = join(room, 'cleanroom_python_assert.py'); + copyFileSync(join(HERE, 'cleanroom_python_assert.py'), probePath); + const argv = [probePath, '--dist', name, '--module', target.import_module]; if (target.import_symbol) argv.push('--symbol', target.import_symbol); const probe = run(py, argv, { cwd: room, timeout: 180000 }); let parsed; diff --git a/scripts/ga/cleanroom_python_assert.py b/scripts/ga/cleanroom_python_assert.py index 1da61f9..bee7769 100644 --- a/scripts/ga/cleanroom_python_assert.py +++ b/scripts/ga/cleanroom_python_assert.py @@ -39,6 +39,69 @@ def check(name: str, ok: bool, detail: str) -> dict: return {"name": name, "ok": bool(ok), "detail": detail} +# Source-root layouts a checkout can present for the module under test. The clean room must +# not be able to satisfy `import ` from ANY of them, and the guard must not be keyed +# on one repository's directory names — that is precisely how it went blind before: +# "" wave-av/sdk-python -> `wave_sdk/` at the repository root +# "sdk-python" wave-av/sdks -> `sdk-python/wave_sdk/` +# "src"/"python" the two other conventional source roots, so a future re-layout of either +# repo does not silently disarm this check again +CHECKOUT_SUBROOTS: tuple[str, ...] = ("", "sdk-python", "src", "python") + + +def interpreter_owned(path: str) -> bool: + """True when a sys.path entry belongs to the interpreter/venv under test. + + site-packages (where the published wheel legitimately installs), the stdlib, and + everything else under the venv's own prefix are not leaks. They must be excluded + explicitly, because the leak test below asks "can this entry supply the module?" and + the correct answer for site-packages is yes. The install roots are read from sysconfig + and `site` rather than assumed to live under `sys.prefix`, so a `--user` install (which + does not) cannot be mistaken for a checkout. + """ + rp = realpath(path) + if not rp: + return False + roots = [realpath(sys.prefix), realpath(getattr(sys, "base_prefix", sys.prefix))] + paths = sysconfig.get_paths() + roots += [realpath(paths[k]) for k in ("purelib", "platlib", "stdlib", "platstdlib") if k in paths] + try: + import site + + roots += [realpath(p) for p in (getattr(site, "USER_SITE", None), getattr(site, "USER_BASE", None)) if p] + except ImportError: # pragma: no cover - `site` is unimportable only under -S + pass + return any(r and (rp == r or rp.startswith(r + os.sep)) for r in roots) + + +def checkout_paths_providing(paths: list[str], module: str) -> list[str]: + """sys.path entries from which a SOURCE CHECKOUT could satisfy `import `. + + Layout-independent on purpose. The previous implementation looked for one hardcoded + path (`/sdk-python/wave`); when `wave` was renamed to `wave_sdk` it matched + nothing anywhere and reported "no repo checkout on sys.path" unconditionally — a guard + that cannot fail, which is worse than no guard, because the whole clean-room result + rests on it. Keying on the module actually under test, across the layouts a checkout + can have, is what makes it self-maintaining. + """ + hits: list[str] = [] + for p in paths: + entry = p or os.getcwd() # '' means cwd, which can absolutely be a checkout + if interpreter_owned(entry): + continue + for sub in CHECKOUT_SUBROOTS: + base = os.path.join(entry, sub) if sub else entry + pkg_dir = os.path.join(base, module) + mod_file = os.path.join(base, module + ".py") + if os.path.isdir(pkg_dir): + hits.append(pkg_dir) + break + if os.path.isfile(mod_file): + hits.append(mod_file) + break + return hits + + def dist_top_level(dist_name: str) -> list[str]: """Top-level import names the installed distribution claims. @@ -84,16 +147,13 @@ def main() -> int: checks: list[dict] = [] # Guard: a repo checkout on sys.path would make this whole run meaningless. - # Keyed on `args.module` (the same name the import check below uses), not a literal - # "wave" — that literal was the pre-rename package directory name, and after the - # `wave` -> `wave_sdk` rename (ART-001, this repo's sdk-python and the sibling - # wave-av/sdk-python repo both moved) a hardcoded "wave" here silently stopped - # matching either checkout's real layout, leaving this guard permanently blind - # to the exact repo-on-sys.path leak it exists to catch. - repo_marker_on_path = [ - p for p in sys.path - if p and os.path.isdir(os.path.join(p, "sdk-python", args.module)) - ] + # Keyed on `args.module` (the same name the import check below uses) and on every + # source-root layout a checkout can present, not on a literal `sdk-python/wave`: + # that literal was the pre-rename package directory of ONE of the two repositories + # this probe runs against, so after the `wave` -> `wave_sdk` rename (ART-001) it + # matched nothing at all and this guard reported a pass unconditionally — blind to + # the exact repo-on-sys.path leak it exists to catch. See checkout_paths_providing(). + repo_marker_on_path = checkout_paths_providing(list(sys.path), args.module) checks.append(check( "cleanroom-isolation", not repo_marker_on_path, diff --git a/sdk-python/tests/test_cleanroom_probe.py b/sdk-python/tests/test_cleanroom_probe.py new file mode 100644 index 0000000..bdb10c5 --- /dev/null +++ b/sdk-python/tests/test_cleanroom_probe.py @@ -0,0 +1,144 @@ +"""Unit guards for the clean-room isolation check in `scripts/ga/cleanroom_python_assert.py`. + +WHY THESE EXIST +--------------- +`cleanroom-isolation` is the load-bearing assertion of the entire registry clean-room gate: +every other check in that probe (`py-import-module`, `py-no-stdlib-shadow`) is only +meaningful if no source checkout can satisfy the import that the PUBLISHED WHEEL is +supposed to satisfy. If the isolation check is wrong, the gate can report a confident PASS +on a package that is completely broken for real users. + +It WAS wrong. The check was keyed on one hardcoded path — `/sdk-python/wave` +— which was the pre-rename layout of exactly one of the two repositories the probe runs +against. After `wave` -> `wave_sdk` (ART-001) it matched nothing on any machine, so it +reported "no repo checkout on sys.path" unconditionally: a check that could not fail. That +is strictly worse than no check, because the rest of the gate's verdict rests on it. + +A hardcoded-path guard has no test that would have caught this, so the fix is not just a +better path list — it is `checkout_paths_providing()`, a pure function over an explicit +sys.path, exercised below against real directory trees for BOTH repository layouts plus the +false-positive case (site-packages, where finding the module is the correct outcome). These +run offline in the normal `pytest` pass, so the guard can never again silently disarm +itself between releases. +""" + +from __future__ import annotations + +import importlib.util +import os +import sys +from pathlib import Path + +import pytest + +PROBE_PATH = Path(__file__).resolve().parents[2] / "scripts" / "ga" / "cleanroom_python_assert.py" + + +def _load_probe(): + """Import the GA probe by path — it is a script, not an installed module.""" + assert PROBE_PATH.is_file(), ( + f"clean-room probe not found at {PROBE_PATH}. If it moved, this test file and " + f".github/workflows/test-python.yml's path filter must move with it." + ) + spec = importlib.util.spec_from_file_location("_ga_cleanroom_probe", PROBE_PATH) + assert spec and spec.loader + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +probe = _load_probe() + + +def _make_checkout(root: Path, subroot: str, module: str) -> Path: + """Create a source-checkout-shaped tree: ///__init__.py.""" + pkg = (root / subroot / module) if subroot else (root / module) + pkg.mkdir(parents=True) + (pkg / "__init__.py").write_text("") + return pkg + + +# --- control: the helper can see anything at all ------------------------------------------ + + +def test_flags_root_level_checkout_layout(tmp_path: Path): + """`wave-av/sdk-python` layout: `wave_sdk/` sits at the repository root. + + The pre-fix guard looked only under `/sdk-python/`, so this — a checkout of the + OTHER repository the probe runs against — was invisible to it. + """ + _make_checkout(tmp_path, "", "wave_sdk") + hits = probe.checkout_paths_providing([str(tmp_path)], "wave_sdk") + assert hits, "a root-level `wave_sdk/` on sys.path must be reported as a clean-room leak" + + +def test_flags_nested_sdk_python_checkout_layout(tmp_path: Path): + """`wave-av/sdks` layout: `sdk-python/wave_sdk/`. This is this repository.""" + _make_checkout(tmp_path, "sdk-python", "wave_sdk") + hits = probe.checkout_paths_providing([str(tmp_path)], "wave_sdk") + assert hits, "a `sdk-python/wave_sdk/` checkout on sys.path must be reported as a leak" + + +def test_flags_single_file_module(tmp_path: Path): + """A leak does not have to be a package — `wave_sdk.py` shadows just as effectively.""" + (tmp_path / "wave_sdk.py").write_text("") + hits = probe.checkout_paths_providing([str(tmp_path)], "wave_sdk") + assert hits, "a top-level `wave_sdk.py` on sys.path must be reported as a leak" + + +def test_flags_the_empty_sys_path_entry_meaning_cwd(tmp_path: Path, monkeypatch): + """`''` on sys.path means the current directory, which can absolutely be a checkout. + + The pre-fix guard skipped falsy entries outright, so `cd` into a checkout and run the + probe and it saw nothing. + """ + _make_checkout(tmp_path, "", "wave_sdk") + monkeypatch.chdir(tmp_path) + hits = probe.checkout_paths_providing([""], "wave_sdk") + assert hits, "the '' sys.path entry (cwd) must be resolved and checked, not skipped" + + +# --- false-positive controls: the guard must stay usable ---------------------------------- + + +def test_does_not_flag_the_interpreters_own_site_packages(): + """Finding the module in site-packages is the CORRECT outcome, not a leak. + + Without this exclusion the guard would fire on every legitimate run, and a guard that + fails when everything is fine gets deleted — which is how gates die. + """ + site_dirs = [p for p in sys.path if "site-packages" in p] + if not site_dirs: + pytest.skip("no site-packages on sys.path in this environment") + for entry in site_dirs: + assert probe.interpreter_owned(entry), ( + f"{entry} is inside the interpreter prefix and must never be treated as a " + f"repository checkout" + ) + assert probe.checkout_paths_providing(site_dirs, "wave_sdk") == [] + + +def test_does_not_flag_an_unrelated_directory(tmp_path: Path): + """A sys.path entry that cannot supply the module under test is not a leak.""" + (tmp_path / "docs").mkdir() + assert probe.checkout_paths_providing([str(tmp_path)], "wave_sdk") == [] + + +def test_guard_is_keyed_on_the_module_under_test(tmp_path: Path): + """The self-maintenance property: rename the package, the guard follows it. + + A checkout of the PRE-rename layout (`wave/`) is not a leak for `import wave_sdk`, and + a checkout of whatever the module is called today always is. This is the assertion the + hardcoded `"wave"` literal could not make, and its absence is why the rename disarmed + the check without a single test going red. + """ + _make_checkout(tmp_path, "sdk-python", "wave") + assert probe.checkout_paths_providing([str(tmp_path)], "wave_sdk") == [] + assert probe.checkout_paths_providing([str(tmp_path)], "wave") != [] + + +def test_reported_hit_points_at_the_offending_path(tmp_path: Path): + """The failure message has to name the leaking path, not just say something is wrong.""" + pkg = _make_checkout(tmp_path, "sdk-python", "wave_sdk") + hits = probe.checkout_paths_providing([str(tmp_path)], "wave_sdk") + assert [os.path.realpath(h) for h in hits] == [os.path.realpath(str(pkg))]