fix(activator): install a bundle root's package only when a declared module lives there; fail by name

amplifier_foundation/bundle/_dataclass.py

@@ -3,26 +3,29 @@
 from __future__ import annotations
 
 import logging
-from dataclasses import dataclass
-from dataclasses import field
+from collections.abc import Callable
+from dataclasses import dataclass, field
 from pathlib import Path
-from typing import TYPE_CHECKING
-from typing import Any
-from typing import Callable
+from typing import TYPE_CHECKING, Any
 
 if TYPE_CHECKING:
     from amplifier_foundation.bundle._prepared import PreparedBundle
 
 from amplifier_foundation.bundle._provenance import (
     _prov_add as _prov_add,  # re-exported for backwards compatibility
+)
+from amplifier_foundation.bundle._provenance import (
     build_initial_provenance,
     capture_existing_ids,
-    tag_container_provenance as tag_container_provenance,  # re-exported for registry
     track_provenance,
 )
-from amplifier_foundation.configurator._types import Origin as Origin  # noqa: F401 re-export
-from amplifier_foundation.dicts.merge import deep_merge
-from amplifier_foundation.dicts.merge import merge_module_lists
+from amplifier_foundation.bundle._provenance import (
+    tag_container_provenance as tag_container_provenance,  # re-exported for registry
+)
+from amplifier_foundation.configurator._types import (
+    Origin as Origin,
+)
+from amplifier_foundation.dicts.merge import deep_merge, merge_module_lists
 from amplifier_foundation.exceptions import BundleValidationError
 from amplifier_foundation.paths.construction import construct_context_path
 
@@ -354,7 +357,10 @@ def resolve_with_overrides(module_id: str, source: str) -> str:
             BundleModuleResolver,
             PreparedBundle,
         )
-        from amplifier_foundation.modules.activator import ModuleActivator
+        from amplifier_foundation.modules.activator import (
+            BundlePackageInstallError,
+            ModuleActivator,
+        )
 
         # Get mount plan
         mount_plan = self.to_mount_plan()
@@ -365,24 +371,6 @@ def resolve_with_overrides(module_id: str, source: str) -> str:
             install_deps=install_deps, base_path=self.base_path, strict=strict
         )
 
-        # CRITICAL: Install bundle packages BEFORE activating modules
-        # Modules may import from their parent bundle's package (e.g., a tool
-        # module importing helpers from `amplifier_bundle_<name>`). These packages
-        # must be installed before modules can be activated.
-        if install_deps:
-            # Install this bundle's package (if it has pyproject.toml)
-            if self.base_path:
-                await activator.activate_bundle_package(
-                    self.base_path, progress_callback=progress_callback
-                )
-
-            # Install packages from all included bundles (from source_base_paths)
-            for namespace, bundle_path in self.source_base_paths.items():
-                if bundle_path and bundle_path != self.base_path:
-                    await activator.activate_bundle_package(
-                        bundle_path, progress_callback=progress_callback
-                    )
-
         # Collect all modules that need activation
         modules_to_activate = []
 
@@ -446,6 +434,48 @@ def resolve_source(mod_spec: dict) -> dict:
         # Warnings are logged but do not fail prepare().
         mode_warnings = self.validate_modes()
 
+        # CRITICAL: Install bundle packages BEFORE activating modules.
+        # Modules may import from their parent bundle's package (e.g., a tool
+        # module importing helpers from `amplifier_bundle_<name>`), so the package
+        # must be present before activate_all(). The decision is made from the
+        # modules just collected: a root is installed only when one of ITS declared
+        # modules resolves inside it. A root pyproject alone is not a reason -- for
+        # an application repo shipping a skills-only behavior it would install the
+        # application into this environment, and fail every session when it can't.
+        if install_deps:
+            declared_sources = [
+                m["source"]
+                for m in modules_to_activate
+                if isinstance(m.get("source"), str)
+            ]
+            # This bundle's own package: a failure here is a failure of the bundle
+            # being prepared, so it propagates (attributed to this root).
+            if self.base_path:
+                await activator.activate_bundle_package(
+                    self.base_path,
+                    progress_callback=progress_callback,
+                    module_sources=declared_sources,
+                )
+            # Included bundles' packages. Honor `strict` exactly as module
+            # activation does: strict raises; otherwise the include's package is
+            # skipped with a warning naming it, and any module that truly needed it
+            # fails on its own, by name, in activate_all().
+            for _namespace, bundle_path in self.source_base_paths.items():
+                if not bundle_path or bundle_path == self.base_path:
+                    continue
+                try:
+                    await activator.activate_bundle_package(
+                        bundle_path,
+                        progress_callback=progress_callback,
+                        module_sources=declared_sources,
+                    )
+                except BundlePackageInstallError as exc:
+                    if strict:
+                        raise
+                    logger.warning(
+                        f"Included bundle '{_namespace}' package skipped: {exc}"
+                    )
+
         # Activate all modules and get their paths
         module_paths = await activator.activate_all(
             modules_to_activate, progress_callback=progress_callback

amplifier_foundation/modules/activator.py

@@ -13,15 +13,16 @@
 import asyncio
 import importlib
 import logging
+import platform
 import site
 import subprocess
 import sys
+from collections.abc import Callable, Iterable
 from pathlib import Path
-from typing import Callable
 
 from amplifier_foundation.exceptions import BundleError
 from amplifier_foundation.modules.install_state import InstallStateManager
-from amplifier_foundation.paths.resolution import get_amplifier_home
+from amplifier_foundation.paths.resolution import get_amplifier_home, parse_uri
 from amplifier_foundation.sources.resolver import SimpleSourceResolver
 
 logger = logging.getLogger(__name__)
@@ -49,6 +50,98 @@ def _distribution_installed(pkg_name: str) -> bool:
         return False
 
 
+class BundlePackageInstallError(BundleError):
+    """A bundle's own root Python package could not be installed.
+
+    Raised by :meth:`ModuleActivator.activate_bundle_package` so the failure names
+    the bundle that OWNS the offending ``pyproject.toml`` -- not whichever bundle
+    happened to be preparing when the install ran. Without this attribution the
+    user sees ``Failed to load bundle 'foundation'`` for a package that belongs to
+    an unrelated ``--app`` bundle they added an hour ago.
+    """
+
+    def __init__(self, bundle_path: Path, package: str, reason: str) -> None:
+        self.bundle_path = bundle_path
+        self.package = package
+        self.reason = reason
+        super().__init__(
+            f"Could not install the root Python package '{package or bundle_path.name}' "
+            f"of bundle at {bundle_path}: {reason}\n"
+            f"That package is installed only because a module declared by the bundle "
+            f"resolves inside that directory. If this bundle was added with "
+            f"`amplifier bundle add`, `amplifier bundle remove <name>` restores sessions."
+        )
+
+
+def bundle_root_declares_module(
+    bundle_path: Path, module_sources: Iterable[str]
+) -> bool:
+    """Does at least one declared module ``source`` resolve INSIDE ``bundle_path``?
+
+    This is the question :meth:`ModuleActivator.activate_bundle_package` exists to
+    serve -- "modules that import from their parent bundle's package" -- asked of
+    the modules actually declared, rather than inferred from the mere presence of a
+    ``pyproject.toml`` with a ``[project]`` table. A skills-only behavior shipped
+    from a Python *application* repo has a ``[project]`` table (the application) but
+    declares no module that lives there; installing the application into the
+    Amplifier environment is never what its author meant, and when the package
+    cannot install (``requires-python`` above the running interpreter) every session
+    on the machine fails at bundle preparation.
+
+    Two source shapes count as "inside":
+
+    * Local paths. Relative ``./`` and ``../`` sources are rewritten to absolute
+      paths at load time (``_dataclass._resolve_relative_sources``), so a plain
+      ``Path(source).resolve().is_relative_to(bundle_path)`` is exact.
+    * ``git+`` sources whose repo AND ref hash to the same cache directory as
+      ``bundle_path`` -- the same pure computation the git handler uses to place
+      clones (``GitSourceHandler._get_cache_path``), evaluated against the cache
+      directory the bundle itself was fetched into (``bundle_path.parent``). A
+      ``#subdirectory=modules/x`` module of the same repo therefore matches; a
+      module fetched from any other repo does not.
+
+    Anything unparseable is treated as "not inside" -- the conservative answer,
+    because the cost of a false positive here is a machine-wide outage while the
+    cost of a false negative is one module failing to import, loudly, by name.
+    """
+    try:
+        root = bundle_path.resolve()
+    except OSError:
+        return False
+    git_handler = None
+    for source in module_sources:
+        if not isinstance(source, str) or not source:
+            continue
+        try:
+            parsed = parse_uri(source)
+        except Exception as exc:  # noqa: BLE001
+            # An unparseable source is simply "not ours" -- the conservative answer.
+            logger.debug(f"Ignoring unparseable module source {source!r}: {exc}")
+            continue
+        if parsed.is_git:
+            if git_handler is None:
+                from amplifier_foundation.sources.git import GitSourceHandler
+
+                git_handler = GitSourceHandler()
+            try:
+                if git_handler._get_cache_path(parsed, root.parent).resolve() == root:
+                    return True
+            except Exception as exc:  # noqa: BLE001
+                logger.debug(
+                    f"Could not place git source {source!r} in the cache: {exc}"
+                )
+            continue
+        if parsed.is_file:
+            raw = source.removeprefix("file://")
+            try:
+                candidate = Path(raw).expanduser().resolve()
+            except (OSError, RuntimeError):
+                continue
+            if candidate == root or candidate.is_relative_to(root):
+                return True
+    return False
+
+
 class ModuleActivator:
     """Activate modules by downloading and making them importable.
 
@@ -210,6 +303,8 @@ async def activate_bundle_package(
         self,
         bundle_path: Path,
         progress_callback: Callable[[str, str], None] | None = None,
+        *,
+        module_sources: Iterable[str] | None = None,
     ) -> None:
         """Install a bundle's own Python package to enable internal imports.
 
@@ -224,6 +319,18 @@ async def activate_bundle_package(
 
         Args:
             bundle_path: Path to bundle root directory containing pyproject.toml.
+            module_sources: The ``source`` strings of every module the bundle
+                declares. When given, the package is installed ONLY if at least
+                one of them resolves inside ``bundle_path`` (see
+                :func:`bundle_root_declares_module`) -- a root ``pyproject.toml``
+                alone is not evidence that any module imports from it. ``None``
+                preserves the historical behavior (install whenever the pyproject
+                declares a package) for callers that cannot supply the list.
+
+        Raises:
+            BundlePackageInstallError: the package's ``requires-python`` excludes
+                the running interpreter, or the install itself failed. Either way
+                the error names THIS bundle root and package.
 
         Note:
             This is a no-op if the bundle has no pyproject.toml.
@@ -254,6 +361,20 @@ async def activate_bundle_package(
             )
             return
 
+        # A [project] table proves the repo ships a Python package. It does not
+        # prove any module in this bundle imports from it -- an application repo
+        # that ships a skills-only behavior has a [project] table for the
+        # application. Only install when a declared module actually lives here.
+        if module_sources is not None and not bundle_root_declares_module(
+            bundle_path, module_sources
+        ):
+            logger.info(
+                f"Skipping root package install for bundle at {bundle_path}: none of the "
+                f"bundle's declared modules resolve inside it, so its pyproject describes "
+                f"an application, not a module dependency."
+            )
+            return
+
         # Skip packages that are already installed in the current environment.
         # This prevents editable-installing packages (like amplifier-core) that were
         # already installed from PyPI as prebuilt wheels. Without this check, a repo
@@ -270,10 +391,48 @@ async def activate_bundle_package(
             )
             return
 
+        # Fail with a sentence, not a resolver transcript: if the package's own
+        # requires-python excludes the interpreter Amplifier runs on, uv will refuse
+        # anyway -- say so first, naming the bundle, before spawning it.
+        requires_python = str(
+            pyproject_data.get("project", {}).get("requires-python", "")
+        ).strip()
+        if requires_python:
+            try:
+                from packaging.specifiers import SpecifierSet
+            except ImportError:
+                # `packaging` is not a declared dependency; without it the check is
+                # skipped and uv's own resolver error is surfaced (attributed) below.
+                SpecifierSet = None  # type: ignore[assignment]
+            if SpecifierSet is not None:
+                running = platform.python_version()
+                if not SpecifierSet(requires_python).contains(
+                    running, prereleases=True
+                ):
+                    raise BundlePackageInstallError(
+                        bundle_path,
+                        pkg_name,
+                        f"it requires Python {requires_python} but this Amplifier "
+                        f"environment runs Python {running}",
+                    )
+
         if progress_callback:
             progress_callback("installing_package", pkg_name or bundle_path.name)
         logger.debug(f"Installing bundle package from {bundle_path}")
-        await self._install_dependencies(bundle_path)
+        try:
+            await self._install_dependencies(bundle_path)
+        except subprocess.CalledProcessError as e:
+            detail = (e.stderr or e.stdout or "").strip()
+            raise BundlePackageInstallError(
+                bundle_path,
+                pkg_name,
+                f"`uv pip install -e` exited {e.returncode}"
+                + (f"\n{detail}" if detail else ""),
+            ) from e
+        except FileNotFoundError as e:
+            raise BundlePackageInstallError(
+                bundle_path, pkg_name, "uv is not installed"
+            ) from e
 
         # CRITICAL: Also add bundle's src/ directory to sys.path explicitly.
         # Editable installs (uv pip install -e) create .pth files or importlib finders,
@@ -315,7 +474,6 @@ def _build_git_dep_overrides(pyproject_path: Path) -> list[str]:
         Returns a list of ``"name==version"`` strings suitable for a uv overrides file.
         """
         import importlib.metadata
-
         import tomllib
 
         try:
@@ -596,5 +754,3 @@ class ModuleActivationError(BundleError):
     preparation failures render this cleanly instead of letting it
     escape as an unhandled traceback.
     """
-
-    pass