From 2e2139786830611d0f5e4496c4f3a0bdd1a523ef Mon Sep 17 00:00:00 2001 From: Albrecht Degering Date: Mon, 20 Jul 2026 20:01:14 +0200 Subject: [PATCH] chore(checks): discover workspace module boundaries --- tests/test_dependency_boundaries.py | 144 +++++++++++++++++ tools/checks/check-dependency-hygiene.sh | 21 +-- tools/checks/check-focused.sh | 23 ++- tools/checks/check-manifest-shapes.py | 113 +++++++++++++ tools/checks/check_dependency_boundaries.py | 167 ++++++++++++-------- 5 files changed, 375 insertions(+), 93 deletions(-) create mode 100644 tests/test_dependency_boundaries.py create mode 100644 tools/checks/check-manifest-shapes.py diff --git a/tests/test_dependency_boundaries.py b/tests/test_dependency_boundaries.py new file mode 100644 index 0000000..d2d1186 --- /dev/null +++ b/tests/test_dependency_boundaries.py @@ -0,0 +1,144 @@ +from __future__ import annotations + +import importlib.util +import json +from pathlib import Path +import sys +import tempfile +import unittest + + +META_ROOT = Path(__file__).resolve().parents[1] +SCRIPT = META_ROOT / "tools" / "checks" / "check_dependency_boundaries.py" + + +def load_boundary_module(): + spec = importlib.util.spec_from_file_location("check_dependency_boundaries", SCRIPT) + if spec is None or spec.loader is None: + raise RuntimeError(f"Could not load {SCRIPT}") + module = importlib.util.module_from_spec(spec) + sys.modules[spec.name] = module + spec.loader.exec_module(module) + return module + + +class DependencyBoundaryDiscoveryTests(unittest.TestCase): + def test_editable_install_metadata_is_not_treated_as_a_source_package(self) -> None: + boundary = load_boundary_module() + with tempfile.TemporaryDirectory(prefix="govoplan-boundary-discovery-") as directory: + root = Path(directory) + package = root / "govoplan-example" / "src" / "govoplan_example" + package.mkdir(parents=True) + (package / "__init__.py").write_text("", encoding="utf-8") + (package.parent / "govoplan_example.egg-info").mkdir() + + repos, prefixes, errors = boundary._discover_python_repos(root) + + self.assertEqual(errors, ()) + self.assertEqual(repos, {"govoplan-example": package}) + self.assertEqual(prefixes, {"govoplan-example": "govoplan_example"}) + + def test_ambiguous_or_missing_source_packages_fail_discovery(self) -> None: + boundary = load_boundary_module() + with tempfile.TemporaryDirectory(prefix="govoplan-boundary-discovery-") as directory: + root = Path(directory) + missing_source = root / "govoplan-missing" / "src" + missing_source.mkdir(parents=True) + (missing_source / "govoplan_missing.egg-info").mkdir() + + ambiguous_source = root / "govoplan-ambiguous" / "src" + for name in ("govoplan_alpha", "govoplan_beta"): + package = ambiguous_source / name + package.mkdir(parents=True) + (package / "__init__.py").write_text("", encoding="utf-8") + + repos, prefixes, errors = boundary._discover_python_repos(root) + + self.assertEqual(repos, {}) + self.assertEqual(prefixes, {}) + self.assertEqual(len(errors), 2) + self.assertTrue(any("govoplan-missing" in error and "found 0" in error for error in errors)) + self.assertTrue(any("govoplan-ambiguous" in error and "found 2" in error for error in errors)) + + def test_duplicate_python_package_ownership_fails_discovery(self) -> None: + boundary = load_boundary_module() + with tempfile.TemporaryDirectory(prefix="govoplan-boundary-discovery-") as directory: + root = Path(directory) + for owner in ("govoplan-first", "govoplan-second"): + package = root / owner / "src" / "govoplan_shared" + package.mkdir(parents=True) + (package / "__init__.py").write_text("", encoding="utf-8") + + repos, prefixes, errors = boundary._discover_python_repos(root) + + self.assertEqual(set(repos), {"govoplan-first"}) + self.assertEqual(prefixes, {"govoplan-first": "govoplan_shared"}) + self.assertEqual(len(errors), 1) + self.assertIn("already owned", errors[0]) + + def test_workspace_discovery_covers_changed_capability_repositories(self) -> None: + boundary = load_boundary_module() + + self.assertEqual(boundary.PYTHON_DISCOVERY_ERRORS, ()) + self.assertGreaterEqual(len(boundary.REPOS), 40) + for owner in ( + "govoplan-core", + "govoplan-identity", + "govoplan-idm", + "govoplan-calendar", + "govoplan-poll", + "govoplan-scheduling", + ): + self.assertIn(owner, boundary.REPOS) + + def test_webui_discovery_is_fail_closed(self) -> None: + boundary = load_boundary_module() + with tempfile.TemporaryDirectory(prefix="govoplan-boundary-webui-") as directory: + root = Path(directory) + good = root / "govoplan-good" / "webui" + good.mkdir(parents=True) + (good / "package.json").write_text( + json.dumps({"name": "@govoplan/good-webui"}), + encoding="utf-8", + ) + missing = root / "govoplan-missing" / "webui" + missing.mkdir(parents=True) + invalid = root / "govoplan-invalid" / "webui" + invalid.mkdir(parents=True) + (invalid / "package.json").write_text("{", encoding="utf-8") + unnamed = root / "govoplan-unnamed" / "webui" + unnamed.mkdir(parents=True) + (unnamed / "package.json").write_text("{}", encoding="utf-8") + duplicate = root / "govoplan-zduplicate" / "webui" + duplicate.mkdir(parents=True) + (duplicate / "package.json").write_text( + json.dumps({"name": "@govoplan/good-webui"}), + encoding="utf-8", + ) + + repos, packages, errors = boundary._discover_webui_repos(root) + + self.assertEqual(repos, {"govoplan-good": good}) + self.assertEqual(packages, {"govoplan-good": "@govoplan/good-webui"}) + self.assertEqual(len(errors), 4) + self.assertTrue(any("govoplan-missing" in error and "no package.json" in error for error in errors)) + self.assertTrue(any("govoplan-invalid" in error and "invalid" in error for error in errors)) + self.assertTrue(any("govoplan-unnamed" in error and "package name" in error for error in errors)) + self.assertTrue(any("govoplan-zduplicate" in error and "already owned" in error for error in errors)) + + def test_workspace_webui_discovery_covers_composition_repositories(self) -> None: + boundary = load_boundary_module() + + self.assertEqual(boundary.WEBUI_DISCOVERY_ERRORS, ()) + self.assertGreaterEqual(len(boundary.WEBUI_REPOS), 17) + for owner in ( + "govoplan-core", + "govoplan-calendar", + "govoplan-scheduling", + "govoplan-notifications", + ): + self.assertIn(owner, boundary.WEBUI_REPOS) + + +if __name__ == "__main__": + unittest.main() diff --git a/tools/checks/check-dependency-hygiene.sh b/tools/checks/check-dependency-hygiene.sh index 2112c1a..58839a2 100644 --- a/tools/checks/check-dependency-hygiene.sh +++ b/tools/checks/check-dependency-hygiene.sh @@ -37,23 +37,10 @@ import pathlib import sys root = pathlib.Path.cwd() -scan_roots = [ - root / "src", - root / "tests", - root.parent / "govoplan-access" / "src", - root.parent / "govoplan-admin" / "src", - root.parent / "govoplan-addresses" / "src", - root.parent / "govoplan-audit" / "src", - root.parent / "govoplan-calendar" / "src", - root.parent / "govoplan-campaign" / "src", - root.parent / "govoplan-dashboard" / "src", - root.parent / "govoplan-files" / "src", - root.parent / "govoplan-identity" / "src", - root.parent / "govoplan-mail" / "src", - root.parent / "govoplan-ops" / "src", - root.parent / "govoplan-organizations" / "src", - root.parent / "govoplan-policy" / "src", - root.parent / "govoplan-tenancy" / "src", +scan_roots = [root / "tests"] + [ + repo / "src" + for repo in sorted(root.parent.glob("govoplan*")) + if (repo / "src").is_dir() ] deprecated_tokens = { "HTTP_422_UNPROCESSABLE_ENTITY": "Use HTTP_422_UNPROCESSABLE_CONTENT; the numeric status remains 422.", diff --git a/tools/checks/check-focused.sh b/tools/checks/check-focused.sh index d8a8fda..30946df 100644 --- a/tools/checks/check-focused.sh +++ b/tools/checks/check-focused.sh @@ -28,27 +28,24 @@ cd "$ROOT" GOVOPLAN_CORE_ROOT="$ROOT" PYTHON="$PYTHON" CHECK_TESTCLIENT_DEPRECATIONS=1 bash "$META_ROOT/tools/checks/check-dependency-hygiene.sh" "$PYTHON" "$META_ROOT/tools/checks/check-contracts.py" --no-impact +PYTHONDONTWRITEBYTECODE=1 "$PYTHON" "$META_ROOT/tools/checks/check-manifest-shapes.py" "$PYTHON" - <<'PY' import ast import pathlib import sys +repos_root = pathlib.Path("/mnt/DATA/git") roots = [ - pathlib.Path("/mnt/DATA/git/govoplan-core/src"), - pathlib.Path("/mnt/DATA/git/govoplan-access/src"), - pathlib.Path("/mnt/DATA/git/govoplan-admin/src"), - pathlib.Path("/mnt/DATA/git/govoplan-addresses/src"), - pathlib.Path("/mnt/DATA/git/govoplan-tenancy/src"), - pathlib.Path("/mnt/DATA/git/govoplan-organizations/src"), - pathlib.Path("/mnt/DATA/git/govoplan-identity/src"), - pathlib.Path("/mnt/DATA/git/govoplan-policy/src"), - pathlib.Path("/mnt/DATA/git/govoplan-audit/src"), - pathlib.Path("/mnt/DATA/git/govoplan-mail/src"), - pathlib.Path("/mnt/DATA/git/govoplan-files/src"), - pathlib.Path("/mnt/DATA/git/govoplan-campaign/src"), - pathlib.Path("/mnt/DATA/git/govoplan-mail/tests"), + repo / "src" + for repo in sorted(repos_root.glob("govoplan*")) + if (repo / "src").is_dir() ] +roots.extend( + repo / "tests" + for repo in sorted(repos_root.glob("govoplan*")) + if (repo / "tests").is_dir() +) errors = [] count = 0 diff --git a/tools/checks/check-manifest-shapes.py b/tools/checks/check-manifest-shapes.py new file mode 100644 index 0000000..c8cff57 --- /dev/null +++ b/tools/checks/check-manifest-shapes.py @@ -0,0 +1,113 @@ +#!/usr/bin/env python3 +"""Load and validate every available GovOPlaN module manifest from source.""" + +from __future__ import annotations + +import argparse +import importlib +import json +import sys +import tomllib +from pathlib import Path + + +META_ROOT = Path(__file__).resolve().parents[2] + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "--workspace-root", + type=Path, + default=None, + help="Directory containing the GovOPlaN repositories. Defaults to repositories.json default_parent.", + ) + args = parser.parse_args() + + catalog = json.loads((META_ROOT / "repositories.json").read_text(encoding="utf-8")) + workspace_root = (args.workspace_root or Path(catalog["default_parent"])).resolve() + repositories = tuple(catalog["repositories"]) + + source_roots: list[Path] = [] + manifest_sources: list[tuple[str, Path, Path]] = [] + for repository in repositories: + repository_root = workspace_root / repository["path"] + source_root = repository_root / "src" + if not source_root.is_dir(): + continue + source_roots.append(source_root) + manifest_sources.extend( + (repository["name"], source_root, manifest_path) + for manifest_path in sorted(source_root.glob("*/backend/manifest.py")) + ) + + if not manifest_sources: + print(f"No module manifests found below {workspace_root}.", file=sys.stderr) + return 1 + + core_source = workspace_root / "govoplan-core" / "src" + ordered_source_roots = [core_source, *(root for root in source_roots if root != core_source)] + sys.path[:0] = [str(root) for root in ordered_source_roots] + + from govoplan_core.core.modules import ModuleManifest # noqa: PLC0415 + from govoplan_core.core.registry import PlatformRegistry, RegistryError # noqa: PLC0415 + + manifests: list[ModuleManifest] = [] + errors: list[str] = [] + for repository_name, source_root, manifest_path in manifest_sources: + module_name = ".".join(manifest_path.relative_to(source_root).with_suffix("").parts) + try: + loaded_module = importlib.import_module(module_name) + get_manifest = getattr(loaded_module, "get_manifest") + manifest = get_manifest() + except Exception as exc: # pragma: no cover - rendered as a check failure + errors.append(f"{repository_name}: could not load {module_name}: {exc}") + continue + + if not isinstance(manifest, ModuleManifest): + errors.append(f"{repository_name}: {module_name}.get_manifest() did not return ModuleManifest") + continue + + entry_points = _module_entry_points(manifest_path.parents[3] / "pyproject.toml") + expected_target = f"{module_name}:get_manifest" + actual_target = entry_points.get(manifest.id) + if actual_target != expected_target: + declared = ", ".join(f"{name}={target}" for name, target in sorted(entry_points.items())) or "none" + errors.append( + f"{repository_name}: module {manifest.id!r} must declare entry point " + f"{manifest.id}={expected_target}; found {declared}" + ) + continue + + manifests.append(manifest) + + if errors: + print("\n".join(errors), file=sys.stderr) + return 1 + + registry = PlatformRegistry() + try: + for manifest in manifests: + registry.register(manifest) + snapshot = registry.validate() + except RegistryError as exc: + print(f"Manifest registry validation failed: {exc}", file=sys.stderr) + return 1 + + print( + f"Manifest registry check passed: {len(snapshot.manifests)} manifests " + f"from {len(manifest_sources)} source files." + ) + return 0 + + +def _module_entry_points(pyproject_path: Path) -> dict[str, str]: + if not pyproject_path.is_file(): + return {} + pyproject = tomllib.loads(pyproject_path.read_text(encoding="utf-8")) + values = pyproject.get("project", {}).get("entry-points", {}).get("govoplan.modules", {}) + return {str(name): str(target) for name, target in values.items()} + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tools/checks/check_dependency_boundaries.py b/tools/checks/check_dependency_boundaries.py index 6adfd78..3241052 100644 --- a/tools/checks/check_dependency_boundaries.py +++ b/tools/checks/check_dependency_boundaries.py @@ -11,52 +11,89 @@ from pathlib import Path META_ROOT = Path(__file__).resolve().parents[2] CORE_ROOT = Path(os.environ.get("GOVOPLAN_CORE_ROOT", META_ROOT.parent / "govoplan-core")).resolve() REPOS_ROOT = Path(os.environ.get("GOVOPLAN_REPOS_ROOT", CORE_ROOT.parent)).resolve() -REPOS = { - "core": CORE_ROOT / "src" / "govoplan_core", - "access": REPOS_ROOT / "govoplan-access" / "src" / "govoplan_access", - "admin": REPOS_ROOT / "govoplan-admin" / "src" / "govoplan_admin", - "addresses": REPOS_ROOT / "govoplan-addresses" / "src" / "govoplan_addresses", - "tenancy": REPOS_ROOT / "govoplan-tenancy" / "src" / "govoplan_tenancy", - "organizations": REPOS_ROOT / "govoplan-organizations" / "src" / "govoplan_organizations", - "identity": REPOS_ROOT / "govoplan-identity" / "src" / "govoplan_identity", - "policy": REPOS_ROOT / "govoplan-policy" / "src" / "govoplan_policy", - "audit": REPOS_ROOT / "govoplan-audit" / "src" / "govoplan_audit", - "dashboard": REPOS_ROOT / "govoplan-dashboard" / "src" / "govoplan_dashboard", - "files": REPOS_ROOT / "govoplan-files" / "src" / "govoplan_files", - "mail": REPOS_ROOT / "govoplan-mail" / "src" / "govoplan_mail", - "campaign": REPOS_ROOT / "govoplan-campaign" / "src" / "govoplan_campaign", -} -PREFIXES = { - "core": "govoplan_core", - "access": "govoplan_access", - "admin": "govoplan_admin", - "addresses": "govoplan_addresses", - "tenancy": "govoplan_tenancy", - "organizations": "govoplan_organizations", - "identity": "govoplan_identity", - "policy": "govoplan_policy", - "audit": "govoplan_audit", - "dashboard": "govoplan_dashboard", - "files": "govoplan_files", - "mail": "govoplan_mail", - "campaign": "govoplan_campaign", -} -FEATURE_OWNERS = ("addresses", "files", "mail", "campaign") -PLATFORM_OWNERS = ("admin", "tenancy", "organizations", "identity", "policy", "audit", "dashboard") -WEBUI_REPOS = { - "core": CORE_ROOT / "webui", - "addresses": REPOS_ROOT / "govoplan-addresses" / "webui", - "files": REPOS_ROOT / "govoplan-files" / "webui", - "mail": REPOS_ROOT / "govoplan-mail" / "webui", - "campaign": REPOS_ROOT / "govoplan-campaign" / "webui", -} -WEBUI_PACKAGES = { - "core": "@govoplan/core-webui", - "addresses": "@govoplan/addresses-webui", - "files": "@govoplan/files-webui", - "mail": "@govoplan/mail-webui", - "campaign": "@govoplan/campaign-webui", -} + + +def _discover_python_repos( + repos_root: Path = REPOS_ROOT, +) -> tuple[dict[str, Path], dict[str, str], tuple[str, ...]]: + repos: dict[str, Path] = {} + prefixes: dict[str, str] = {} + errors: list[str] = [] + for repo_root in sorted(repos_root.glob("govoplan*")): + source_root = repo_root / "src" + if not source_root.is_dir(): + continue + packages = sorted( + path + for path in source_root.glob("govoplan_*") + if path.is_dir() and (path / "__init__.py").is_file() + ) + if len(packages) != 1: + discovered = ", ".join(path.name for path in packages) or "none" + errors.append( + f"{repo_root.name}: expected exactly one importable govoplan_* package " + f"under {source_root}, found {len(packages)} ({discovered})" + ) + continue + owner = repo_root.name + prefix = packages[0].name + duplicate_owner = next( + (candidate_owner for candidate_owner, candidate_prefix in prefixes.items() if candidate_prefix == prefix), + None, + ) + if duplicate_owner is not None: + errors.append( + f"{owner}: import package {prefix!r} is already owned by {duplicate_owner}; " + "Python package ownership must be unique" + ) + continue + repos[owner] = packages[0] + prefixes[owner] = prefix + return repos, prefixes, tuple(errors) + + +def _discover_webui_repos( + repos_root: Path = REPOS_ROOT, +) -> tuple[dict[str, Path], dict[str, str], tuple[str, ...]]: + repos: dict[str, Path] = {} + packages: dict[str, str] = {} + errors: list[str] = [] + for repo_root in sorted(repos_root.glob("govoplan*")): + webui_root = repo_root / "webui" + if not webui_root.is_dir(): + continue + package_path = webui_root / "package.json" + if not package_path.is_file(): + errors.append(f"{repo_root.name}: WebUI directory has no package.json: {package_path}") + continue + try: + payload = json.loads(package_path.read_text(encoding="utf-8")) + except json.JSONDecodeError as exc: + errors.append(f"{repo_root.name}: invalid {package_path}: {exc}") + continue + package_name = payload.get("name") if isinstance(payload, dict) else None + if not isinstance(package_name, str) or not package_name: + errors.append(f"{repo_root.name}: {package_path} must declare a non-empty package name") + continue + owner = repo_root.name + duplicate_owner = next( + (candidate_owner for candidate_owner, candidate_package in packages.items() if candidate_package == package_name), + None, + ) + if duplicate_owner is not None: + errors.append( + f"{owner}: WebUI package {package_name!r} is already owned by {duplicate_owner}; " + "WebUI package ownership must be unique" + ) + continue + repos[owner] = webui_root + packages[owner] = package_name + return repos, packages, tuple(errors) + + +REPOS, PREFIXES, PYTHON_DISCOVERY_ERRORS = _discover_python_repos() +WEBUI_REPOS, WEBUI_PACKAGES, WEBUI_DISCOVERY_ERRORS = _discover_webui_repos() +CORE_OWNER = CORE_ROOT.name WEBUI_IMPORT_RE = re.compile( r"(?:from\s+|import\s*\(\s*|require\s*\(\s*|export\s+[^;]*?\s+from\s+)" r"[\"'](?P@govoplan/[a-z0-9_-]+-webui)(?:/[A-Za-z0-9_./-]+)?[\"']" @@ -127,18 +164,7 @@ def _violations() -> list[Violation]: imported_owner = _import_owner(imported) if imported_owner is None or imported_owner == owner: continue - if owner == "core" and imported_owner in FEATURE_OWNERS: - if not _allowed(owner, path, imported_owner): - violations.append(Violation("python", owner, path, lineno, imported, imported_owner)) - elif owner == "access" and imported_owner in FEATURE_OWNERS: - violations.append(Violation("python", owner, path, lineno, imported, imported_owner)) - elif owner in PLATFORM_OWNERS and imported_owner == "access": - if not _allowed(owner, path, imported_owner): - violations.append(Violation("python", owner, path, lineno, imported, imported_owner)) - elif owner in FEATURE_OWNERS and imported_owner in FEATURE_OWNERS: - if not _allowed(owner, path, imported_owner): - violations.append(Violation("python", owner, path, lineno, imported, imported_owner)) - elif owner in FEATURE_OWNERS and imported_owner == "access": + if imported_owner != CORE_OWNER and not _allowed(owner, path, imported_owner): violations.append(Violation("python", owner, path, lineno, imported, imported_owner)) violations.extend(_webui_violations()) return violations @@ -191,7 +217,12 @@ def _webui_package_json_violations(owner: str, root: Path) -> list[Violation]: imported_owner = _webui_package_owner(str(package_name)) if imported_owner is None or imported_owner == owner: continue - if owner in FEATURE_OWNERS and imported_owner in FEATURE_OWNERS: + # The core WebUI package is also the development composition host; + # package declarations make independently built module frontends + # available to Vite without authorizing source-level imports. + if owner == CORE_OWNER: + continue + if imported_owner != CORE_OWNER: violations.append(Violation("webui-package", owner, package_path, 1, str(package_name), imported_owner)) return violations @@ -206,9 +237,7 @@ def _webui_source_violations(owner: str, root: Path) -> list[Violation]: if imported_owner is None or imported_owner == owner: continue lineno = text.count("\n", 0, match.start()) + 1 - if owner == "core" and imported_owner in FEATURE_OWNERS: - violations.append(Violation("webui-source", owner, path, lineno, package_name, imported_owner)) - elif owner in FEATURE_OWNERS and imported_owner in FEATURE_OWNERS: + if imported_owner != CORE_OWNER: violations.append(Violation("webui-source", owner, path, lineno, package_name, imported_owner)) return violations @@ -224,9 +253,21 @@ def _webui_violations() -> list[Violation]: def main() -> int: + discovery_errors = (*PYTHON_DISCOVERY_ERRORS, *WEBUI_DISCOVERY_ERRORS) + if discovery_errors: + print("Dependency boundary discovery errors:") + for error in discovery_errors: + print(f"- {error}") + print("\nThe boundary scan is incomplete; fix package discovery before accepting this gate.") + return 1 + violations = _violations() if not violations: - print(f"Dependency boundary check passed ({len(ALLOWLIST)} transitional exceptions tracked)") + print( + "Dependency boundary check passed " + f"({len(REPOS)} Python repos, {len(WEBUI_REPOS)} WebUI repos, " + f"{len(ALLOWLIST)} transitional exceptions tracked)" + ) return 0 print("Dependency boundary violations:")