From 81178b7be322aa4d968e7f2c280653c3208dc00c Mon Sep 17 00:00:00 2001 From: cproudlock Date: Sat, 11 Jul 2026 10:36:28 -0400 Subject: [PATCH] Make the import-surface scan cover symlinked external plugins Path.rglob does not descend symlinks, so a symlinked external plugin (the ADR-003 dev loop) silently escaped the contract-purity scan. The scanner now resolves plugin dirs before walking, a regression test plants a symlinked plugin with a real violation and asserts it is flagged, and the known-limitation notes in the external-repo docs are lifted. Co-Authored-By: Claude Fable 5 --- docs/PLUGIN-EXTERNAL-REPO.md | 13 ++++++----- scripts/test-external-plugin.sh | 12 +++++----- tests/test_plugin_contract.py | 41 +++++++++++++++++++++++++++++++-- 3 files changed, 52 insertions(+), 14 deletions(-) diff --git a/docs/PLUGIN-EXTERNAL-REPO.md b/docs/PLUGIN-EXTERNAL-REPO.md index 72470e2..5b4c610 100644 --- a/docs/PLUGIN-EXTERNAL-REPO.md +++ b/docs/PLUGIN-EXTERNAL-REPO.md @@ -266,12 +266,13 @@ manifest parses, the `core_version` range admits the framework's contract test run errors and the script exits nonzero), models expose `__tablename__`, and hooks return the right shapes. -One gap to know about: the framework's `test_plugins_only_import_contract_surface` -scans `plugins/` with `Path.rglob`, which does not descend symlinked directories -on CPython 3.12. So that particular sub-test does not see a symlinked external -plugin's source. Keep an equivalent import-surface assertion in your own -`tests/` so your CI still enforces "core imports only via `shopdb.api`". A -minimal version: +The import-surface scan (`test_plugins_only_import_contract_surface`) covers +symlinked plugins too: the scanner resolves each plugin directory before +walking it, because `Path.rglob` alone does not descend symlinks (pinned by +`test_import_scan_covers_symlinked_plugins`). You can additionally keep an +equivalent import-surface assertion in your own `tests/`, so violations fail +in your repo's CI even when run without the framework harness. A minimal +version: ```python # tests/test_import_surface.py diff --git a/scripts/test-external-plugin.sh b/scripts/test-external-plugin.sh index 7764135..396e764 100755 --- a/scripts/test-external-plugin.sh +++ b/scripts/test-external-plugin.sh @@ -32,12 +32,12 @@ # # Offline: test a bundled plugin against this checkout, no network # LOCAL_FRAMEWORK=. PLUGIN_DIR=plugins/warranty scripts/test-external-plugin.sh # -# Note on coverage: a symlinked plugin is discovered and loaded by the plugin -# loader (so loadability, manifest validity, and the core_version range are all -# checked), but the import-surface scan in test_plugin_contract.py uses -# Path.rglob over plugins/, which does not descend symlinked directories on -# CPython 3.12. To have that scan cover your plugin too, keep an equivalent -# import-surface assertion in your own tests/ (see docs/PLUGIN-EXTERNAL-REPO.md). +# Coverage: a symlinked plugin is discovered and loaded by the plugin loader +# (loadability, manifest validity, core_version range) AND scanned by the +# import-surface contract test - the scanner resolves each plugin dir before +# walking, since bare Path.rglob does not descend symlinks. Optionally keep an +# equivalent import-surface assertion in your own tests/ so violations fail +# in your repo's CI too (see docs/PLUGIN-EXTERNAL-REPO.md). set -eu diff --git a/tests/test_plugin_contract.py b/tests/test_plugin_contract.py index f629130..6613306 100644 --- a/tests/test_plugin_contract.py +++ b/tests/test_plugin_contract.py @@ -209,8 +209,17 @@ def _plugin_source_files(): # Skip migrations/ - Alembic env.py is framework glue that legitimately # calls the shared runner in shopdb.plugins.alembic_template (see ADR-008), # not plugin domain code. Mirrors the style check excluding versions/. - return [p for p in root.rglob('*.py') - if '__pycache__' not in p.parts and 'migrations' not in p.parts] + # Resolve each plugin dir before rglob: rglob does not descend symlinked + # directories, and external plugins arrive as symlinks per ADR-003. + files = [p for p in root.glob('*.py')] + for plugindir in sorted(root.iterdir()): + if not plugindir.is_dir(): + continue + for path in plugindir.resolve().rglob('*.py'): + if '__pycache__' in path.parts or 'migrations' in path.parts: + continue + files.append(path) + return files def test_plugins_only_import_contract_surface(): @@ -231,3 +240,31 @@ def test_plugins_only_import_contract_surface(): 'Plugins must import core only via shopdb.api or shopdb.plugins.base. ' 'Violations:\n' + '\n'.join(violations) ) + + +def test_import_scan_covers_symlinked_plugins(tmp_path): + """External plugins symlinked into plugins/ ARE scanned for violations. + + Path.rglob does not descend symlinked directories, so a naive scan would + silently skip external plugins installed per ADR-003 (clone or symlink + into plugins//). The scanner resolves each plugin dir first; this + test plants a symlinked plugin with a violating import and asserts the + scan both sees the file and flags the violation. + """ + root = Path(__file__).resolve().parent.parent / 'plugins' + external = tmp_path / 'symlinkdemo' + external.mkdir() + # violating import: internal core path, not the contract surface + (external / 'plugin.py').write_text('from shopdb.extensions import db\n') + + link = root / 'zzsymlinkdemo' + link.symlink_to(external, target_is_directory=True) + try: + scanned = {str(p) for p in _plugin_source_files()} + assert any('symlinkdemo' in s for s in scanned), ( + 'symlinked plugin sources were not scanned' + ) + with pytest.raises(AssertionError, match='shopdb.extensions'): + test_plugins_only_import_contract_surface() + finally: + link.unlink()