From a55d8b9b60aca2b5021ad1576667cb54f6e33333 Mon Sep 17 00:00:00 2001 From: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> Date: Wed, 9 Sep 2026 07:58:50 -0700 Subject: [PATCH 1/5] fix(validation): reuse canonical module during path checks Generated with Amplifier Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- python/amplifier_core/validation/base.py | 47 +++++ python/amplifier_core/validation/context.py | 31 ++- python/amplifier_core/validation/hook.py | 31 ++- .../amplifier_core/validation/orchestrator.py | 31 ++- python/amplifier_core/validation/provider.py | 31 ++- python/amplifier_core/validation/tool.py | 31 ++- tests/test_validation_canonical_module.py | 176 ++++++++++++++++++ 7 files changed, 278 insertions(+), 100 deletions(-) create mode 100644 tests/test_validation_canonical_module.py diff --git a/python/amplifier_core/validation/base.py b/python/amplifier_core/validation/base.py index 6a3cbdc..4e84e24 100644 --- a/python/amplifier_core/validation/base.py +++ b/python/amplifier_core/validation/base.py @@ -11,9 +11,13 @@ ``context/release-mandate.md`` for the v1.4.0 regression that motivated this. """ +import importlib.util import inspect +import sys from dataclasses import dataclass from dataclasses import field +from pathlib import Path +from types import ModuleType from typing import Any from typing import Literal @@ -62,6 +66,49 @@ def summary(self) -> str: return f"{status}: {passed_count}/{len(self.checks)} checks passed ({len(self.errors)} errors, {len(self.warnings)} warnings)" +def import_module_from_path(module_path: str | Path) -> ModuleType: + """Import a Python source path without duplicating its canonical module. + + When runtime loading already imported the module from the same source, + validation must inspect that object. A same-named module from another + source is deliberately not reused: path validation must validate the + requested file rather than whichever package happens to be in + ``sys.modules``. + """ + path = Path(module_path) + source_path = path / "__init__.py" if path.is_dir() else path + module_name = path.name if path.is_dir() else path.stem + + existing = sys.modules.get(module_name) + if existing is not None: + existing_file = getattr(existing, "__file__", None) + if existing_file is not None and Path(existing_file).resolve() == source_path.resolve(): + return existing + + spec = importlib.util.spec_from_file_location(module_name, source_path) + if spec is None or spec.loader is None: + raise ImportError(f"Could not load spec for {path}") + + module = importlib.util.module_from_spec(spec) + module_prefix = f"{module_name}." + previous_modules = { + name: value + for name, value in sys.modules.items() + if name == module_name or name.startswith(module_prefix) + } + for name in previous_modules: + del sys.modules[name] + sys.modules[module_name] = module + try: + spec.loader.exec_module(module) + finally: + for name in list(sys.modules): + if name == module_name or name.startswith(module_prefix): + del sys.modules[name] + sys.modules.update(previous_modules) + return module + + def check_on_session_ready(module: Any) -> ValidationCheck | None: """Check whether a module's on_session_ready() function, if present, is valid. diff --git a/python/amplifier_core/validation/context.py b/python/amplifier_core/validation/context.py index 5440783..2f15059 100644 --- a/python/amplifier_core/validation/context.py +++ b/python/amplifier_core/validation/context.py @@ -8,7 +8,6 @@ import asyncio import importlib -import importlib.util import inspect from pathlib import Path from typing import Any @@ -16,6 +15,7 @@ from .base import ValidationCheck from .base import ValidationResult from .base import check_on_session_ready +from .base import import_module_from_path def _implements_context_manager_interface(obj: Any) -> bool: @@ -95,11 +95,7 @@ def _check_importable( # File path - find the Python module if path.is_dir(): init_file = path / "__init__.py" - if init_file.exists(): - spec = importlib.util.spec_from_file_location( - path.name, init_file - ) - else: + if not init_file.exists(): result.add( ValidationCheck( name="module_importable", @@ -109,21 +105,16 @@ def _check_importable( ) ) return None - else: - spec = importlib.util.spec_from_file_location(path.stem, path) - - if spec and spec.loader: - module = importlib.util.module_from_spec(spec) - spec.loader.exec_module(module) - result.add( - ValidationCheck( - name="module_importable", - passed=True, - message=f"Module loaded from {path}", - severity="info", - ) + module = import_module_from_path(path) + result.add( + ValidationCheck( + name="module_importable", + passed=True, + message=f"Module loaded from {path}", + severity="info", ) - return module + ) + return module else: # Module name - import directly module = importlib.import_module(str(module_path)) diff --git a/python/amplifier_core/validation/hook.py b/python/amplifier_core/validation/hook.py index e3e68ee..5f2b41e 100644 --- a/python/amplifier_core/validation/hook.py +++ b/python/amplifier_core/validation/hook.py @@ -8,7 +8,6 @@ import asyncio import importlib -import importlib.util import inspect from pathlib import Path from typing import Any @@ -16,6 +15,7 @@ from .base import ValidationCheck from .base import ValidationResult from .base import check_on_session_ready +from .base import import_module_from_path def _implements_hook_handler_interface(obj: Any) -> bool: @@ -86,11 +86,7 @@ def _check_importable( # File path - find the Python module if path.is_dir(): init_file = path / "__init__.py" - if init_file.exists(): - spec = importlib.util.spec_from_file_location( - path.name, init_file - ) - else: + if not init_file.exists(): result.add( ValidationCheck( name="module_importable", @@ -100,21 +96,16 @@ def _check_importable( ) ) return None - else: - spec = importlib.util.spec_from_file_location(path.stem, path) - - if spec and spec.loader: - module = importlib.util.module_from_spec(spec) - spec.loader.exec_module(module) - result.add( - ValidationCheck( - name="module_importable", - passed=True, - message=f"Module loaded from {path}", - severity="info", - ) + module = import_module_from_path(path) + result.add( + ValidationCheck( + name="module_importable", + passed=True, + message=f"Module loaded from {path}", + severity="info", ) - return module + ) + return module else: # Module name - import directly module = importlib.import_module(str(module_path)) diff --git a/python/amplifier_core/validation/orchestrator.py b/python/amplifier_core/validation/orchestrator.py index 09a6e98..0a722f3 100644 --- a/python/amplifier_core/validation/orchestrator.py +++ b/python/amplifier_core/validation/orchestrator.py @@ -8,7 +8,6 @@ import asyncio import importlib -import importlib.util import inspect from pathlib import Path from typing import Any @@ -16,6 +15,7 @@ from .base import ValidationCheck from .base import ValidationResult from .base import check_on_session_ready +from .base import import_module_from_path def _implements_orchestrator_interface(obj: Any) -> bool: @@ -86,11 +86,7 @@ def _check_importable( # File path - find the Python module if path.is_dir(): init_file = path / "__init__.py" - if init_file.exists(): - spec = importlib.util.spec_from_file_location( - path.name, init_file - ) - else: + if not init_file.exists(): result.add( ValidationCheck( name="module_importable", @@ -100,21 +96,16 @@ def _check_importable( ) ) return None - else: - spec = importlib.util.spec_from_file_location(path.stem, path) - - if spec and spec.loader: - module = importlib.util.module_from_spec(spec) - spec.loader.exec_module(module) - result.add( - ValidationCheck( - name="module_importable", - passed=True, - message=f"Module loaded from {path}", - severity="info", - ) + module = import_module_from_path(path) + result.add( + ValidationCheck( + name="module_importable", + passed=True, + message=f"Module loaded from {path}", + severity="info", ) - return module + ) + return module else: # Module name - import directly module = importlib.import_module(str(module_path)) diff --git a/python/amplifier_core/validation/provider.py b/python/amplifier_core/validation/provider.py index 3626f8a..8066f63 100644 --- a/python/amplifier_core/validation/provider.py +++ b/python/amplifier_core/validation/provider.py @@ -8,7 +8,6 @@ import asyncio import importlib -import importlib.util import inspect from pathlib import Path from typing import Any @@ -17,6 +16,7 @@ from .base import ValidationCheck from .base import ValidationResult from .base import check_on_session_ready +from .base import import_module_from_path def _implements_provider_interface(obj: Any) -> bool: @@ -96,11 +96,7 @@ def _check_importable( # File path - find the Python module if path.is_dir(): init_file = path / "__init__.py" - if init_file.exists(): - spec = importlib.util.spec_from_file_location( - path.name, init_file - ) - else: + if not init_file.exists(): result.add( ValidationCheck( name="module_importable", @@ -110,21 +106,16 @@ def _check_importable( ) ) return None - else: - spec = importlib.util.spec_from_file_location(path.stem, path) - - if spec and spec.loader: - module = importlib.util.module_from_spec(spec) - spec.loader.exec_module(module) - result.add( - ValidationCheck( - name="module_importable", - passed=True, - message=f"Module loaded from {path}", - severity="info", - ) + module = import_module_from_path(path) + result.add( + ValidationCheck( + name="module_importable", + passed=True, + message=f"Module loaded from {path}", + severity="info", ) - return module + ) + return module else: # Module name - import directly module = importlib.import_module(str(module_path)) diff --git a/python/amplifier_core/validation/tool.py b/python/amplifier_core/validation/tool.py index e4bfe02..a10c7c4 100644 --- a/python/amplifier_core/validation/tool.py +++ b/python/amplifier_core/validation/tool.py @@ -8,7 +8,6 @@ import asyncio import importlib -import importlib.util import inspect from pathlib import Path from typing import Any @@ -16,6 +15,7 @@ from .base import ValidationCheck from .base import ValidationResult from .base import check_on_session_ready +from .base import import_module_from_path def _implements_tool_interface(obj: Any) -> bool: @@ -91,11 +91,7 @@ def _check_importable( # File path - find the Python module if path.is_dir(): init_file = path / "__init__.py" - if init_file.exists(): - spec = importlib.util.spec_from_file_location( - path.name, init_file - ) - else: + if not init_file.exists(): result.add( ValidationCheck( name="module_importable", @@ -105,21 +101,16 @@ def _check_importable( ) ) return None - else: - spec = importlib.util.spec_from_file_location(path.stem, path) - - if spec and spec.loader: - module = importlib.util.module_from_spec(spec) - spec.loader.exec_module(module) - result.add( - ValidationCheck( - name="module_importable", - passed=True, - message=f"Module loaded from {path}", - severity="info", - ) + module = import_module_from_path(path) + result.add( + ValidationCheck( + name="module_importable", + passed=True, + message=f"Module loaded from {path}", + severity="info", ) - return module + ) + return module else: # Module name - import directly module = importlib.import_module(str(module_path)) diff --git a/tests/test_validation_canonical_module.py b/tests/test_validation_canonical_module.py new file mode 100644 index 0000000..96d6184 --- /dev/null +++ b/tests/test_validation_canonical_module.py @@ -0,0 +1,176 @@ +"""Regression tests for validation sharing the runtime module object.""" + +import importlib +import sys +from pathlib import Path + +import pytest + +from amplifier_core.loader import ModuleLoader +from amplifier_core.validation import ToolValidator + + +class _Source: + def __init__(self, root: Path) -> None: + self.root = root + + def resolve(self) -> Path: + return self.root + + +class _Resolver: + def __init__(self, root: Path) -> None: + self.source = _Source(root) + + def resolve( + self, + module_id: str, + source_hint: str | dict | None = None, + profile_hint: str | dict | None = None, + ) -> _Source: + return self.source + + +class _ResolverCoordinator: + def __init__(self, root: Path) -> None: + self.resolver = _Resolver(root) + + def get(self, mount_point: str) -> _Resolver: + assert mount_point == "module-source-resolver" + return self.resolver + + +class _RecordingCoordinator: + def __init__(self) -> None: + self.mounted: dict[str, object] = {} + + async def mount(self, mount_point: str, module: object, name: str | None = None) -> None: + assert mount_point == "tools" + assert name is not None + self.mounted[name] = module + + +def _write_tool_package(root: Path, package_name: str, token: str) -> Path: + package = root / package_name + package.mkdir(parents=True) + (package / "marker.py").write_text(f'TOKEN = "{token}"\n') + (package / "__init__.py").write_text( + f""" +from .marker import TOKEN + +IMPORT_COUNT = globals().get("IMPORT_COUNT", 0) + 1 +MODULE_TOKEN = object() +__amplifier_module_type__ = "tool" + +class StatefulTool: + name = "stateful" + description = TOKEN + module_token = MODULE_TOKEN + + async def execute(self, input): + return {{}} + +async def mount(coordinator, config): + await coordinator.mount("tools", StatefulTool(), name="stateful") +""" + ) + return package + + +@pytest.mark.asyncio +async def test_loader_validation_and_runtime_share_canonical_package( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Validation must inspect the package the filesystem loader later mounts.""" + module_id = "tool-canonical" + package_name = "amplifier_module_tool_canonical" + package = _write_tool_package(tmp_path, package_name, "canonical") + seen: dict[str, object] = {} + original = ToolValidator._check_mount_exists + + def capture_validated_module(self, result, module): + seen["module"] = module + return original(self, result, module) + + monkeypatch.setattr(ToolValidator, "_check_mount_exists", capture_validated_module) + loader = ModuleLoader(coordinator=_ResolverCoordinator(tmp_path)) + monkeypatch.setattr(loader, "_load_entry_point", lambda _module_id: None) + + try: + mount = await loader.load(module_id) + canonical = sys.modules[package_name] + + assert seen["module"] is canonical + assert canonical.IMPORT_COUNT == 1 + + runtime_coordinator = _RecordingCoordinator() + await mount(runtime_coordinator) + assert runtime_coordinator.mounted["stateful"].module_token is canonical.MODULE_TOKEN + assert canonical.IMPORT_COUNT == 1 + finally: + loader.cleanup() + sys.modules.pop(package_name, None) + + +@pytest.mark.asyncio +async def test_path_validation_does_not_reuse_same_named_different_source( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A canonical package from another path must not substitute for the target.""" + package_name = "amplifier_module_tool_collision" + first_root = tmp_path / "first" + second_root = tmp_path / "second" + _write_tool_package(first_root, package_name, "first") + second_package = _write_tool_package(second_root, package_name, "second") + seen: dict[str, object] = {} + original = ToolValidator._check_mount_exists + + def capture_validated_module(self, result, module): + seen["module"] = module + return original(self, result, module) + + monkeypatch.setattr(ToolValidator, "_check_mount_exists", capture_validated_module) + monkeypatch.syspath_prepend(str(first_root)) + + try: + canonical = importlib.import_module(package_name) + result = await ToolValidator().validate(second_package) + + assert result.passed + assert seen["module"] is not canonical + assert Path(seen["module"].__file__).resolve() == ( + second_package / "__init__.py" + ).resolve() + assert seen["module"].StatefulTool.description == "second" + finally: + sys.modules.pop(package_name, None) + + +@pytest.mark.asyncio +async def test_path_only_validation_imports_requested_package(tmp_path: Path) -> None: + """Standalone path validation remains valid when no package is loaded.""" + package_name = "amplifier_module_tool_standalone" + package = _write_tool_package(tmp_path, package_name, "standalone") + + try: + result = await ToolValidator().validate(package) + assert result.passed + assert package_name not in sys.modules + finally: + sys.modules.pop(package_name, None) + + +@pytest.mark.asyncio +async def test_path_validation_rejects_malformed_module(tmp_path: Path) -> None: + """Reusing canonical modules does not turn malformed source into a pass.""" + package = tmp_path / "amplifier_module_tool_malformed" + package.mkdir() + (package / "__init__.py").write_text("async def mount(:\n") + + result = await ToolValidator().validate(package) + + assert not result.passed + assert any( + check.name == "module_importable" and not check.passed + for check in result.checks + ) \ No newline at end of file From cbc20bcb64be05d5f08e3d2e3bf3b19955d16d0d Mon Sep 17 00:00:00 2001 From: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> Date: Wed, 9 Sep 2026 08:37:08 -0700 Subject: [PATCH 2/5] fix(loader): isolate source-resolved module imports Generated with Amplifier Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- python/amplifier_core/loader.py | 83 +++--- python/amplifier_core/validation/base.py | 111 +++++--- tests/test_validation_canonical_module.py | 295 ++++++++++++++++++++-- 3 files changed, 403 insertions(+), 86 deletions(-) diff --git a/python/amplifier_core/loader.py b/python/amplifier_core/loader.py index ef6f064..694bada 100644 --- a/python/amplifier_core/loader.py +++ b/python/amplifier_core/loader.py @@ -73,6 +73,7 @@ def __init__( search_paths: Optional list of filesystem paths for direct discovery """ self._loaded_modules: dict[str, Any] = {} + self._loaded_module_paths: dict[str, Path] = {} self._module_info: dict[str, ModuleInfo] = {} self._search_paths = search_paths self._coordinator = coordinator @@ -204,17 +205,43 @@ async def load( if module_id in self._loaded_modules: logger.debug(f"Module '{module_id}' already loaded, creating fresh closure") raw_fn = self._loaded_modules[module_id] + cached_source_path = self._loaded_module_paths.get(module_id) + source_resolver = None + if self._coordinator: + with contextlib.suppress(ValueError): + source_resolver = self._coordinator.get("module-source-resolver") + if cached_source_path is not None and self._coordinator: + if source_resolver is not None: + if hasattr(source_resolver, "async_resolve"): + source = await source_resolver.async_resolve( + module_id, + source_hint=source_hint, + profile_hint=source_hint, + ) + else: + source = source_resolver.resolve( + module_id, + source_hint=source_hint, + profile_hint=source_hint, + ) + requested_source_path = source.resolve().resolve() + if requested_source_path != cached_source_path: + raise ImportError( + f"Refusing to load '{module_id}' from {requested_source_path}: " + f"it is already loaded from {cached_source_path}" + ) - async def mount_with_config_cached( - coordinator: ModuleCoordinator, fn=raw_fn - ): - return await fn(coordinator, config or {}) + if source_resolver is None or cached_source_path is not None: + async def mount_with_config_cached( + coordinator: ModuleCoordinator, fn=raw_fn + ): + return await fn(coordinator, config or {}) - # B1: propagate __on_session_ready__ to fresh closure - if on_sr := getattr(raw_fn, "__on_session_ready__", None): - setattr(mount_with_config_cached, "__on_session_ready__", on_sr) + # B1: propagate __on_session_ready__ to fresh closure + if on_sr := getattr(raw_fn, "__on_session_ready__", None): + setattr(mount_with_config_cached, "__on_session_ready__", on_sr) - return mount_with_config_cached + return mount_with_config_cached try: # Resolve module source @@ -304,7 +331,9 @@ async def mount_with_config_cached( ) # Validate module before loading (Python modules only at this point) - await self._validate_module(module_id, module_path, config=config) + package_path = await self._validate_module( + module_id, module_path, config=config + ) except Exception as resolve_error: # Import here to avoid circular dependency @@ -320,26 +349,13 @@ async def mount_with_config_cached( return mount_fn raise resolve_error - # Try to load via entry point first - raw_fn = self._load_entry_point(module_id) - if raw_fn: - self._loaded_modules[module_id] = raw_fn - - async def mount_with_config_ep( - coordinator: ModuleCoordinator, fn=raw_fn - ): - return await fn(coordinator, config or {}) - - # B1: propagate __on_session_ready__ to closure - if on_sr := getattr(raw_fn, "__on_session_ready__", None): - setattr(mount_with_config_ep, "__on_session_ready__", on_sr) - - return mount_with_config_ep - - # Try filesystem loading - raw_fn = self._load_filesystem(module_id) + # Source resolution selected and validated this filesystem package. + # Do not let an installed entry point for the same module id mount a + # different source after validation. + raw_fn = self._load_filesystem(module_id, module_name=package_path.name) if raw_fn: self._loaded_modules[module_id] = raw_fn + self._loaded_module_paths[module_id] = module_path.resolve() async def mount_with_config_fs( coordinator: ModuleCoordinator, fn=raw_fn @@ -457,7 +473,9 @@ def _load_entry_point(self, module_id: str) -> Callable | None: return None - def _load_filesystem(self, module_id: str) -> Callable | None: + def _load_filesystem( + self, module_id: str, module_name: str | None = None + ) -> Callable | None: """Resolve module from filesystem and return the raw mount function. Returns the raw (un-configured) mount function so callers can cache it @@ -465,7 +483,7 @@ def _load_filesystem(self, module_id: str) -> Callable | None: """ try: # Try to import the module - module_name = f"amplifier_module_{module_id.replace('-', '_')}" + module_name = module_name or f"amplifier_module_{module_id.replace('-', '_')}" module = importlib.import_module(module_name) # Detect on_session_ready lifecycle hook if present. @@ -524,7 +542,7 @@ def _get_module_metadata( package_path = self._find_package_dir(module_id, module_path) if package_path: # Import the module temporarily - module_name = f"amplifier_module_{module_id.replace('-', '_')}" + module_name = package_path.name # Add to sys.path temporarily for import path_str = str(module_path) @@ -603,7 +621,7 @@ def _guess_from_naming( async def _validate_module( self, module_id: str, module_path: Path, config: dict[str, Any] | None = None - ) -> None: + ) -> Path: """ Validate a module before loading. @@ -643,7 +661,7 @@ async def _validate_module( logger.warning( f"Unknown module type '{module_type}' for '{module_id}', skipping validation" ) - return + return module_path # Find the actual Python package directory within the module root # Module structure: amplifier-module-xyz/ contains amplifier_module_xyz/ @@ -664,6 +682,7 @@ async def _validate_module( ) logger.info(f"[module:validated] {module_id} - {result.summary()}") + return package_path def _find_package_dir(self, module_id: str, module_path: Path) -> Path | None: """ diff --git a/python/amplifier_core/validation/base.py b/python/amplifier_core/validation/base.py index 4e84e24..2e72853 100644 --- a/python/amplifier_core/validation/base.py +++ b/python/amplifier_core/validation/base.py @@ -11,7 +11,8 @@ ``context/release-mandate.md`` for the v1.4.0 regression that motivated this. """ -import importlib.util +import _imp +import importlib import inspect import sys from dataclasses import dataclass @@ -67,45 +68,91 @@ def summary(self) -> str: def import_module_from_path(module_path: str | Path) -> ModuleType: - """Import a Python source path without duplicating its canonical module. + """Import a Python source path through Python's normal import machinery. - When runtime loading already imported the module from the same source, - validation must inspect that object. A same-named module from another - source is deliberately not reused: path validation must validate the - requested file rather than whichever package happens to be in - ``sys.modules``. + Validation must use the canonical module object that runtime loading will + mount. If another source already owns the same package name, fail closed + rather than replacing entries in ``sys.modules`` while another importer can + observe them. """ path = Path(module_path) source_path = path / "__init__.py" if path.is_dir() else path module_name = path.name if path.is_dir() else path.stem - existing = sys.modules.get(module_name) - if existing is not None: - existing_file = getattr(existing, "__file__", None) - if existing_file is not None and Path(existing_file).resolve() == source_path.resolve(): - return existing - - spec = importlib.util.spec_from_file_location(module_name, source_path) - if spec is None or spec.loader is None: - raise ImportError(f"Could not load spec for {path}") - - module = importlib.util.module_from_spec(spec) - module_prefix = f"{module_name}." - previous_modules = { - name: value - for name, value in sys.modules.items() - if name == module_name or name.startswith(module_prefix) - } - for name in previous_modules: - del sys.modules[name] - sys.modules[module_name] = module + import_root = path.parent if path.is_dir() else source_path.parent + parent = str(import_root) + _imp.acquire_lock() try: - spec.loader.exec_module(module) + existing = sys.modules.get(module_name) + if existing is not None: + existing_file = getattr(existing, "__file__", None) + if ( + existing_file is not None + and Path(existing_file).resolve() == source_path.resolve() + ): + return existing + raise ImportError( + f"Refusing to import '{module_name}' from {source_path}: " + f"it is already loaded from {existing_file}" + ) + + package_dir = source_path.parent.resolve() + for cached_name, cached_module in sys.modules.items(): + if not cached_name.startswith(f"{module_name}."): + continue + cached_file = getattr(cached_module, "__file__", None) + if cached_file is None or not Path(cached_file).resolve().is_relative_to( + package_dir + ): + raise ImportError( + f"Refusing to import '{module_name}' from {source_path}: " + f"cached submodule '{cached_name}' is from {cached_file}" + ) + + try: + original_path_index = sys.path.index(parent) + except ValueError: + original_path_index = None + next_path = ( + sys.path[original_path_index + 1] + if original_path_index is not None + and original_path_index + 1 < len(sys.path) + else None + ) + if original_path_index is None: + sys.path.insert(0, parent) + elif original_path_index != 0: + sys.path.pop(original_path_index) + sys.path.insert(0, parent) + try: + module = importlib.import_module(module_name) + finally: + current_path_index = next( + (index for index, value in enumerate(sys.path) if value is parent), + None, + ) + if original_path_index is None: + if current_path_index is not None: + sys.path.pop(current_path_index) + elif original_path_index != 0 and current_path_index is not None: + sys.path.pop(current_path_index) + next_path_index = next( + (index for index, value in enumerate(sys.path) if value is next_path), + None, + ) + if next_path_index is None: + sys.path.append(parent) + else: + sys.path.insert(next_path_index, parent) finally: - for name in list(sys.modules): - if name == module_name or name.startswith(module_prefix): - del sys.modules[name] - sys.modules.update(previous_modules) + _imp.release_lock() + + imported_file = getattr(module, "__file__", None) + if imported_file is None or Path(imported_file).resolve() != source_path.resolve(): + raise ImportError( + f"Refusing to validate '{module_name}' from {source_path}: " + f"Python imported {imported_file}" + ) return module diff --git a/tests/test_validation_canonical_module.py b/tests/test_validation_canonical_module.py index 96d6184..9f6604f 100644 --- a/tests/test_validation_canonical_module.py +++ b/tests/test_validation_canonical_module.py @@ -1,13 +1,18 @@ """Regression tests for validation sharing the runtime module object.""" import importlib +import importlib.metadata import sys +import threading +import types from pathlib import Path import pytest from amplifier_core.loader import ModuleLoader +from amplifier_core.loader import ModuleValidationError from amplifier_core.validation import ToolValidator +from amplifier_core.validation.base import import_module_from_path class _Source: @@ -77,14 +82,39 @@ async def mount(coordinator, config): return package +def _clear_package(package_name: str) -> None: + for name in list(sys.modules): + if name == package_name or name.startswith(f"{package_name}."): + sys.modules.pop(name, None) + + +def _write_conflicting_entry_point(root: Path, module_id: str) -> None: + (root / "installed_entry_point.py").write_text( + """ +async def mount(coordinator, config): + await coordinator.mount("tools", object(), name="installed-entry-point") +""" + ) + dist_info = root / "conflicting_entry_point-1.0.dist-info" + dist_info.mkdir() + (dist_info / "METADATA").write_text("Name: conflicting-entry-point\nVersion: 1.0\n") + (dist_info / "entry_points.txt").write_text( + f"[amplifier.modules]\n{module_id} = installed_entry_point:mount\n" + ) + + @pytest.mark.asyncio async def test_loader_validation_and_runtime_share_canonical_package( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: - """Validation must inspect the package the filesystem loader later mounts.""" + """A source-resolved module wins over an installed conflicting entry point.""" module_id = "tool-canonical" package_name = "amplifier_module_tool_canonical" - package = _write_tool_package(tmp_path, package_name, "canonical") + source_root = tmp_path / "source" + package = _write_tool_package(source_root, package_name, "canonical") + entry_point_root = tmp_path / "entry-point" + entry_point_root.mkdir() + _write_conflicting_entry_point(entry_point_root, module_id) seen: dict[str, object] = {} original = ToolValidator._check_mount_exists @@ -93,10 +123,14 @@ def capture_validated_module(self, result, module): return original(self, result, module) monkeypatch.setattr(ToolValidator, "_check_mount_exists", capture_validated_module) - loader = ModuleLoader(coordinator=_ResolverCoordinator(tmp_path)) - monkeypatch.setattr(loader, "_load_entry_point", lambda _module_id: None) + monkeypatch.syspath_prepend(str(entry_point_root)) + loader = ModuleLoader(coordinator=_ResolverCoordinator(source_root)) try: + assert any( + entry_point.name == module_id + for entry_point in importlib.metadata.entry_points(group="amplifier.modules") + ) mount = await loader.load(module_id) canonical = sys.modules[package_name] @@ -106,44 +140,168 @@ def capture_validated_module(self, result, module): runtime_coordinator = _RecordingCoordinator() await mount(runtime_coordinator) assert runtime_coordinator.mounted["stateful"].module_token is canonical.MODULE_TOKEN + assert "installed-entry-point" not in runtime_coordinator.mounted + assert "installed_entry_point" not in sys.modules assert canonical.IMPORT_COUNT == 1 finally: loader.cleanup() - sys.modules.pop(package_name, None) + _clear_package(package_name) @pytest.mark.asyncio -async def test_path_validation_does_not_reuse_same_named_different_source( +async def test_path_validation_rejects_same_named_different_source_without_cache_mutation( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: - """A canonical package from another path must not substitute for the target.""" + """A conflicting source fails without replacing the canonical package tree.""" package_name = "amplifier_module_tool_collision" first_root = tmp_path / "first" second_root = tmp_path / "second" _write_tool_package(first_root, package_name, "first") second_package = _write_tool_package(second_root, package_name, "second") - seen: dict[str, object] = {} - original = ToolValidator._check_mount_exists - def capture_validated_module(self, result, module): - seen["module"] = module - return original(self, result, module) - - monkeypatch.setattr(ToolValidator, "_check_mount_exists", capture_validated_module) monkeypatch.syspath_prepend(str(first_root)) try: canonical = importlib.import_module(package_name) + canonical_marker = sys.modules[f"{package_name}.marker"] result = await ToolValidator().validate(second_package) - assert result.passed - assert seen["module"] is not canonical - assert Path(seen["module"].__file__).resolve() == ( - second_package / "__init__.py" + assert not result.passed + assert any( + check.name == "module_importable" + and not check.passed + and "Refusing to import" in check.message + for check in result.checks + ) + assert sys.modules[package_name] is canonical + assert sys.modules[f"{package_name}.marker"] is canonical_marker + assert canonical.StatefulTool.description == "first" + finally: + _clear_package(package_name) + + +@pytest.mark.asyncio +async def test_path_validation_rejects_a_conflicting_cached_submodule( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A dangling child cache cannot be mixed into a requested package source.""" + package_name = "amplifier_module_tool_submodule_collision" + first_root = tmp_path / "first" + second_root = tmp_path / "second" + _write_tool_package(first_root, package_name, "first") + second_package = _write_tool_package(second_root, package_name, "second") + monkeypatch.syspath_prepend(str(first_root)) + + try: + importlib.import_module(package_name) + cached_marker = sys.modules[f"{package_name}.marker"] + sys.modules.pop(package_name) + + result = await ToolValidator().validate(second_package) + + assert not result.passed + assert any( + check.name == "module_importable" + and not check.passed + and "cached submodule" in check.message + for check in result.checks + ) + assert package_name not in sys.modules + assert sys.modules[f"{package_name}.marker"] is cached_marker + assert cached_marker.TOKEN == "first" + finally: + _clear_package(package_name) + + +@pytest.mark.asyncio +async def test_source_resolved_cache_rejects_a_different_source( + tmp_path: Path, +) -> None: + """A cached source-resolved module cannot silently mount a later source.""" + module_id = "tool-cache-collision" + package_name = "amplifier_module_tool_cache_collision" + first_root = tmp_path / "first" + second_root = tmp_path / "second" + _write_tool_package(first_root, package_name, "first") + _write_tool_package(second_root, package_name, "second") + coordinator = _ResolverCoordinator(first_root) + loader = ModuleLoader(coordinator=coordinator) + + try: + await loader.load(module_id) + coordinator.resolver.source.root = second_root + + with pytest.raises(ImportError, match="already loaded from"): + await loader.load(module_id) + + assert Path(sys.modules[package_name].__file__).resolve() == ( + first_root / package_name / "__init__.py" ).resolve() - assert seen["module"].StatefulTool.description == "second" finally: - sys.modules.pop(package_name, None) + loader.cleanup() + _clear_package(package_name) + + +@pytest.mark.asyncio +async def test_direct_cache_does_not_override_a_later_source_resolution( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A direct cache cannot silently satisfy a later conflicting source request.""" + module_id = "tool-direct-cache" + package_name = "amplifier_module_tool_direct_cache" + direct_root = tmp_path / "direct" + source_root = tmp_path / "source" + _write_tool_package(direct_root, package_name, "direct") + _write_tool_package(source_root, package_name, "source") + monkeypatch.syspath_prepend(str(direct_root)) + loader = ModuleLoader() + + try: + await loader.load(module_id) + loader._coordinator = _ResolverCoordinator(source_root) + + with pytest.raises(ModuleValidationError, match="Refusing to import"): + await loader.load(module_id) + + assert Path(sys.modules[package_name].__file__).resolve() == ( + direct_root / package_name / "__init__.py" + ).resolve() + finally: + loader.cleanup() + _clear_package(package_name) + + +@pytest.mark.asyncio +async def test_source_resolved_fallback_package_is_mounted_from_validated_source( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A fallback package name remains the identity used for runtime mounting.""" + module_id = "tool-beta" + package_name = "amplifier_module_alpha" + package = _write_tool_package(tmp_path, package_name, "source") + installed_root = tmp_path / "installed" + installed_package = installed_root / "amplifier_module_tool_beta" + installed_package.mkdir(parents=True) + (installed_package / "__init__.py").write_text( + '__amplifier_module_type__ = "provider"\n' + ) + monkeypatch.syspath_prepend(str(installed_root)) + loader = ModuleLoader(coordinator=_ResolverCoordinator(tmp_path)) + + try: + mount = await loader.load(module_id) + + runtime_coordinator = _RecordingCoordinator() + await mount(runtime_coordinator) + assert runtime_coordinator.mounted["stateful"].description == "source" + assert "amplifier_module_tool_beta" not in sys.modules + assert Path(sys.modules[package_name].__file__).resolve() == ( + package / "__init__.py" + ).resolve() + finally: + loader.cleanup() + _clear_package(package_name) + _clear_package("amplifier_module_tool_beta") @pytest.mark.asyncio @@ -155,9 +313,102 @@ async def test_path_only_validation_imports_requested_package(tmp_path: Path) -> try: result = await ToolValidator().validate(package) assert result.passed - assert package_name not in sys.modules + assert Path(sys.modules[package_name].__file__).resolve() == ( + package / "__init__.py" + ).resolve() + assert sys.modules[f"{package_name}.marker"].TOKEN == "standalone" + finally: + _clear_package(package_name) + + +def test_standalone_path_validation_prioritizes_requested_source( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """The requested path wins even when it already trails another source path.""" + package_name = "amplifier_module_tool_path_priority" + installed_root = tmp_path / "installed" + requested_root = tmp_path / "requested" + _write_tool_package(installed_root, package_name, "installed") + requested_package = _write_tool_package(requested_root, package_name, "requested") + injected_path = str(tmp_path / "added-by-module") + init_file = requested_package / "__init__.py" + init_file.write_text( + f"import sys\nsys.path.insert(0, {injected_path!r})\n" + init_file.read_text() + ) + monkeypatch.syspath_prepend(str(installed_root)) + sys.path.append(str(requested_root)) + original_path = list(sys.path) + + try: + module = import_module_from_path(requested_package) + + assert module.StatefulTool.description == "requested" + assert sys.path == [injected_path, *original_path] + finally: + _clear_package(package_name) + sys.path.remove(injected_path) + sys.path.remove(str(requested_root)) + + +def test_standalone_path_validation_does_not_expose_temporary_import_path( + tmp_path: Path, +) -> None: + """Normal imports block until the standalone validation path is removed.""" + package_name = "amplifier_module_tool_blocking" + package = tmp_path / package_name + package.mkdir() + entered = threading.Event() + release = threading.Event() + synchronizer = types.ModuleType("_validation_import_synchronizer") + synchronizer.entered = entered + synchronizer.release = release + sys.modules[synchronizer.__name__] = synchronizer + (package / "__init__.py").write_text( + """ +from _validation_import_synchronizer import entered, release + +entered.set() +assert release.wait(timeout=5) +""" + ) + (tmp_path / "unrelated.py").write_text("VALUE = 'must-not-import'\n") + validation_error: list[Exception] = [] + unrelated_result: list[object] = [] + unrelated_done = threading.Event() + + def validate() -> None: + try: + import_module_from_path(package) + except Exception as error: + validation_error.append(error) + + def import_unrelated() -> None: + try: + unrelated_result.append(importlib.import_module("unrelated")) + except Exception as error: + unrelated_result.append(error) + finally: + unrelated_done.set() + + validation_thread = threading.Thread(target=validate) + validation_thread.start() + assert entered.wait(timeout=5) + unrelated_thread = threading.Thread(target=import_unrelated) + unrelated_thread.start() + assert not unrelated_done.wait(timeout=0.1) + release.set() + validation_thread.join(timeout=5) + unrelated_thread.join(timeout=5) + + try: + assert not validation_thread.is_alive() + assert not unrelated_thread.is_alive() + assert not validation_error + assert isinstance(unrelated_result[0], ModuleNotFoundError) finally: - sys.modules.pop(package_name, None) + _clear_package(package_name) + sys.modules.pop(synchronizer.__name__, None) + sys.modules.pop("unrelated", None) @pytest.mark.asyncio From fec390665cde46d989205ef8379a1b910e2134e5 Mon Sep 17 00:00:00 2001 From: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> Date: Sat, 12 Sep 2026 06:47:56 -0700 Subject: [PATCH 3/5] fix(loader): preserve canonical source module identity Generated with Amplifier Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- python/amplifier_core/validation/base.py | 32 +++++++------- tests/test_validation_canonical_module.py | 53 +++++++++++++++++++++++ 2 files changed, 70 insertions(+), 15 deletions(-) diff --git a/python/amplifier_core/validation/base.py b/python/amplifier_core/validation/base.py index 2e72853..bf125fe 100644 --- a/python/amplifier_core/validation/base.py +++ b/python/amplifier_core/validation/base.py @@ -83,6 +83,21 @@ def import_module_from_path(module_path: str | Path) -> ModuleType: parent = str(import_root) _imp.acquire_lock() try: + # Audit children even when the matching root package is already loaded: + # a canonical root must not hide a child cached from a different source. + package_dir = source_path.parent.resolve() + for cached_name, cached_module in list(sys.modules.items()): + if not cached_name.startswith(f"{module_name}."): + continue + cached_file = getattr(cached_module, "__file__", None) + if cached_file is None or not Path(cached_file).resolve().is_relative_to( + package_dir + ): + raise ImportError( + f"Refusing to import '{module_name}' from {source_path}: " + f"cached submodule '{cached_name}' is from {cached_file}" + ) + existing = sys.modules.get(module_name) if existing is not None: existing_file = getattr(existing, "__file__", None) @@ -96,19 +111,6 @@ def import_module_from_path(module_path: str | Path) -> ModuleType: f"it is already loaded from {existing_file}" ) - package_dir = source_path.parent.resolve() - for cached_name, cached_module in sys.modules.items(): - if not cached_name.startswith(f"{module_name}."): - continue - cached_file = getattr(cached_module, "__file__", None) - if cached_file is None or not Path(cached_file).resolve().is_relative_to( - package_dir - ): - raise ImportError( - f"Refusing to import '{module_name}' from {source_path}: " - f"cached submodule '{cached_name}' is from {cached_file}" - ) - try: original_path_index = sys.path.index(parent) except ValueError: @@ -128,7 +130,7 @@ def import_module_from_path(module_path: str | Path) -> ModuleType: module = importlib.import_module(module_name) finally: current_path_index = next( - (index for index, value in enumerate(sys.path) if value is parent), + (index for index, value in enumerate(sys.path) if value == parent), None, ) if original_path_index is None: @@ -137,7 +139,7 @@ def import_module_from_path(module_path: str | Path) -> ModuleType: elif original_path_index != 0 and current_path_index is not None: sys.path.pop(current_path_index) next_path_index = next( - (index for index, value in enumerate(sys.path) if value is next_path), + (index for index, value in enumerate(sys.path) if value == next_path), None, ) if next_path_index is None: diff --git a/tests/test_validation_canonical_module.py b/tests/test_validation_canonical_module.py index 9f6604f..e4aa8d4 100644 --- a/tests/test_validation_canonical_module.py +++ b/tests/test_validation_canonical_module.py @@ -242,6 +242,37 @@ async def test_source_resolved_cache_rejects_a_different_source( _clear_package(package_name) +@pytest.mark.asyncio +async def test_matching_root_with_foreign_cached_child_cannot_mount( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A cached canonical root does not exempt its children from source checks.""" + module_id = "tool-mixed-cache" + package_name = "amplifier_module_tool_mixed_cache" + source_root = tmp_path / "source" + _write_tool_package(source_root, package_name, "canonical") + foreign_package = _write_tool_package(tmp_path / "foreign", package_name, "foreign") + monkeypatch.syspath_prepend(str(source_root)) + loader = ModuleLoader(coordinator=_ResolverCoordinator(source_root)) + + try: + canonical = importlib.import_module(package_name) + foreign_child = types.ModuleType(f"{package_name}.marker") + foreign_child.__file__ = str(foreign_package / "marker.py") + sys.modules[f"{package_name}.marker"] = foreign_child + + with pytest.raises(ModuleValidationError, match="cached submodule"): + await loader.load(module_id) + + assert sys.modules[package_name] is canonical + assert sys.modules[f"{package_name}.marker"] is foreign_child + assert module_id not in loader._loaded_modules + assert canonical.IMPORT_COUNT == 1 + finally: + loader.cleanup() + _clear_package(package_name) + + @pytest.mark.asyncio async def test_direct_cache_does_not_override_a_later_source_resolution( tmp_path: Path, monkeypatch: pytest.MonkeyPatch @@ -350,6 +381,28 @@ def test_standalone_path_validation_prioritizes_requested_source( sys.path.remove(str(requested_root)) +def test_standalone_path_validation_removes_reconstructed_import_path( + tmp_path: Path, +) -> None: + """Cleanup works when the imported package recreates sys.path strings.""" + package_name = "amplifier_module_tool_path_reconstruction" + import_root = tmp_path / "requested" + package = import_root / package_name + package.mkdir(parents=True) + (package / "__init__.py").write_text( + "import sys\n" + "sys.path[:] = [entry.encode().decode() for entry in sys.path]\n" + ) + + try: + module = import_module_from_path(package) + + assert module.__name__ == package_name + assert str(import_root) not in sys.path + finally: + _clear_package(package_name) + + def test_standalone_path_validation_does_not_expose_temporary_import_path( tmp_path: Path, ) -> None: From 98429cd08f3b6d56dd4e04a06f310d759ac03f69 Mon Sep 17 00:00:00 2001 From: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> Date: Sat, 12 Sep 2026 06:47:58 -0700 Subject: [PATCH 4/5] chore: bump core version to 1.6.2 Generated with Amplifier Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- Cargo.lock | 4 ++-- bindings/python/Cargo.toml | 2 +- crates/amplifier-core/Cargo.toml | 2 +- pyproject.toml | 2 +- python/amplifier_core/__init__.py | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b8ae9b1..7e88905 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -40,7 +40,7 @@ checksum = "e9d4ee0d472d1cd2e28c97dfa124b3d8d992e10eb0a035f33f5d12e3a177ba3b" [[package]] name = "amplifier-core" -version = "1.6.1" +version = "1.6.2" dependencies = [ "chrono", "log", @@ -76,7 +76,7 @@ dependencies = [ [[package]] name = "amplifier-core-py" -version = "1.6.1" +version = "1.6.2" dependencies = [ "amplifier-core", "log", diff --git a/bindings/python/Cargo.toml b/bindings/python/Cargo.toml index d43c48b..d4084a3 100644 --- a/bindings/python/Cargo.toml +++ b/bindings/python/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "amplifier-core-py" -version = "1.6.1" +version = "1.6.2" edition = "2021" description = "PyO3 bridge for amplifier-core Rust kernel" license = "MIT" diff --git a/crates/amplifier-core/Cargo.toml b/crates/amplifier-core/Cargo.toml index fadbe06..fcfdde8 100644 --- a/crates/amplifier-core/Cargo.toml +++ b/crates/amplifier-core/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "amplifier-core" -version = "1.6.1" +version = "1.6.2" edition = "2021" description = "Pure Rust kernel for the Amplifier modular AI agent system" license = "MIT" diff --git a/pyproject.toml b/pyproject.toml index 194e602..a7c12bb 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "amplifier-core" -version = "1.6.1" +version = "1.6.2" description = "Rust kernel with Python bindings for the Amplifier modular AI agent framework" license = "MIT" readme = "README.md" diff --git a/python/amplifier_core/__init__.py b/python/amplifier_core/__init__.py index b4e014c..fb4be33 100644 --- a/python/amplifier_core/__init__.py +++ b/python/amplifier_core/__init__.py @@ -6,7 +6,7 @@ AmplifierSession`) still give the pure-Python implementations. """ -__version__ = "1.6.1" +__version__ = "1.6.2" # --- Rust-backed primary types (THE SWITCHOVER) --- # These four were previously imported from their Python submodules. From 6a8bd95b02873695c60d78a63fc2dbc422e95288 Mon Sep 17 00:00:00 2001 From: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> Date: Sat, 12 Sep 2026 07:43:59 -0700 Subject: [PATCH 5/5] chore: synchronize Python lockfile with core 1.6.2 Generated with Amplifier Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- uv.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/uv.lock b/uv.lock index f883d87..e3d45b7 100644 --- a/uv.lock +++ b/uv.lock @@ -4,7 +4,7 @@ requires-python = ">=3.11" [[package]] name = "amplifier-core" -version = "1.5.1" +version = "1.6.2" source = { editable = "." } dependencies = [ { name = "click" },