Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 61 additions & 1 deletion amplifier_app_cli/lib/bundle_loader/resolvers.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
from typing import Protocol
from typing import runtime_checkable

from amplifier_core.module_sources import ModuleNotFoundError as CoreModuleNotFoundError
from amplifier_foundation.paths.resolution import get_amplifier_home
from amplifier_foundation.sources import SimpleSourceResolver

Expand Down Expand Up @@ -451,6 +452,18 @@ def resolve(self, module_id: str, hint: Any = None) -> Any:
...


# The kernel's ``amplifier_core.module_sources.ModuleNotFoundError`` is NOT a
# subclass of the builtin ``ModuleNotFoundError`` (it derives from ``Exception``
# directly). Foundation's ``BundleModuleResolver`` raises the kernel class, so an
# ``except ModuleNotFoundError:`` written against the builtin let it escape --
# and the settings fallback below never ran for a module the bundle did not
# pre-activate. Catch both, so the fallback policy actually applies.
_MODULE_NOT_FOUND: tuple[type[BaseException], ...] = (
ModuleNotFoundError,
CoreModuleNotFoundError,
)


class AppModuleResolver:
"""Composes bundle resolver with settings-based fallback.

Expand Down Expand Up @@ -515,7 +528,7 @@ def resolve(
# Try bundle first (primary source)
try:
return self._bundle.resolve(module_id, hint)
except ModuleNotFoundError:
except _MODULE_NOT_FOUND:
pass # Fall through to settings resolver

# Try settings resolver (fallback)
Expand All @@ -536,6 +549,53 @@ def resolve(
f"Ensure the module is included in the bundle or configure a provider in settings."
)

async def async_resolve(
self, module_id: str, source_hint: Any = None, profile_hint: Any = None
) -> Any:
"""Resolve with the bundle resolver's lazy activation, then the same fallback.

The kernel loader prefers ``async_resolve`` when a resolver exposes it
(amplifier_core/loader.py) and only then can a module that was NOT
pre-activated at ``Bundle.prepare()`` time still load: foundation's
``BundleModuleResolver.async_resolve`` activates such a module on demand
from its ``source_hint`` (clone, install deps, register in ``_paths``).

Without this method the loader fell to the sync ``resolve()`` above, where
the wrapped bundle resolver can only answer for already-activated modules
-- so any hook/tool that reaches the mount plan AFTER prepare() (the
settings-injected ``hooks-routing`` from ``routing.matrix`` is the
measured case) failed at mount with "not found in prepared bundle" even
though its mount-plan entry carried a perfectly good ``source``. This
wrapper is mounted as the session's ``module-source-resolver`` at root
AND inherited by every child session, so the gap was total.

Policy is unchanged: bundle first, settings second, informative error.
"""
hint = profile_hint if profile_hint is not None else source_hint

bundle_async = getattr(self._bundle, "async_resolve", None)
try:
if callable(bundle_async):
return await bundle_async(module_id, source_hint=hint)
return self._bundle.resolve(module_id, hint)
except _MODULE_NOT_FOUND:
pass # Fall through to settings resolver

if self._settings is not None:
try:
result = self._settings.resolve(module_id, hint)
logger.debug(f"Resolved '{module_id}' from settings fallback")
return result
except Exception as e:
logger.debug(f"Settings fallback failed for '{module_id}': {e}")

available = list(getattr(self._bundle, "_paths", {}).keys())
raise ModuleNotFoundError(
f"Module '{module_id}' not found in bundle or user settings. "
f"Bundle contains: {available}. "
f"Ensure the module is included in the bundle or configure a provider in settings."
)

def get_module_source(self, module_id: str) -> str | None:
"""Get module source path as string.

Expand Down
50 changes: 50 additions & 0 deletions amplifier_app_cli/runtime/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from amplifier_core.utils.truncate import SENSITIVE_KEYS
from rich.console import Console

from ..lib.bundle_loader.discovery import WELL_KNOWN_BUNDLES
from ..lib.settings import AppSettings, NotificationFlags, get_custom_routing_dir
from ..lib.merge_utils import merge_module_items
from ..lib.merge_utils import merge_tool_configs
Expand Down Expand Up @@ -291,6 +292,36 @@ def _on_progress(action: str, detail: str) -> None:
**hooks_routing_extra,
**routing_hook_override["config"],
}
# A hook the bundle does not already carry needs a `source`, or the
# module cannot load at all. `_apply_hook_overrides` APPENDS an
# override whose module is absent from the bundle's hooks list, and
# the appended dict is the mount-plan entry verbatim -- so without a
# source here, `hooks-routing` reaches the session as
# `{"module": "hooks-routing", "config": {...}}` and the kernel fails
# it by name at mount:
#
# Failed to load hook 'hooks-routing': Module 'hooks-routing' not
# found in prepared bundle. Available modules: [...]
#
# Measured on a bundle that does not include routing-matrix
# (`anchors-amp-dev`) with `routing.matrix: anthropic` in
# settings.yaml: the routing banner left the system prompt, the
# delegate tool dropped its `model_role` parameter (no
# `model_role_resolver` capability), and every `model_role` fell
# through to the default provider. Silent -- the user sees no
# routing, not an error.
#
# Only attached when the bundle has no hooks-routing of its own: the
# in-place merge path in `_apply_hook_overrides` lets the override's
# top-level keys win (merge_module_items: "child overrides parent,
# including 'source'"), so attaching unconditionally would clobber a
# bundle's deliberately pinned source. The URI is the routing-matrix
# bundle's canonical remote from WELL_KNOWN_BUNDLES -- the same one
# `amplifier routing` and `amplifier update` fetch -- narrowed to the
# hook module's subdirectory, matching the bundle's own
# behaviors/routing.yaml declaration.
if not _bundle_declares_hook(bundle_config.get("hooks"), "hooks-routing"):
routing_hook_override["source"] = _routing_hook_source()
if routing_hook_override["config"]:
hook_overrides.append(routing_hook_override)

Expand Down Expand Up @@ -680,6 +711,25 @@ def narrow_overrides_to_secrets(
return narrowed


def _routing_hook_source() -> str:
"""Canonical source URI for the ``hooks-routing`` module.

Derived from the routing-matrix bundle's registered remote so the CLI has
exactly one place that knows where that bundle lives (``amplifier routing``
and ``amplifier update`` read the same entry). The ``#subdirectory``
fragment mirrors the bundle's own ``behaviors/routing.yaml``.
"""
remote = str(WELL_KNOWN_BUNDLES["routing-matrix"]["remote"])
return f"{remote}#subdirectory=modules/hooks-routing"


def _bundle_declares_hook(hooks: Any, module_id: str) -> bool:
"""True when *hooks* (a bundle's hooks list, possibly absent) names *module_id*."""
if not isinstance(hooks, list):
return False
return any(isinstance(h, dict) and h.get("module") == module_id for h in hooks)


def _apply_hook_overrides(
hooks: list[dict[str, Any]], overrides: list[dict[str, Any]]
) -> list[dict[str, Any]]:
Expand Down
133 changes: 132 additions & 1 deletion tests/lib/bundle_loader/test_resolvers.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,13 @@
"""

from pathlib import Path
from unittest.mock import MagicMock
from unittest.mock import AsyncMock, MagicMock

import pytest

from amplifier_core.module_sources import (
ModuleNotFoundError as CoreModuleNotFoundError,
)
from amplifier_app_cli.lib.bundle_loader.resolvers import (
AppModuleResolver,
FoundationFileSource,
Expand Down Expand Up @@ -363,3 +366,131 @@ def test_get_module_source_falls_back_to_settings(self):
result = app_resolver.get_module_source("my-module")

assert result == "/path/to/module"


class TestAppModuleResolverLazyActivation:
"""A module that reaches the mount plan AFTER Bundle.prepare() -- e.g. the
settings-injected hooks-routing -- is not in the bundle resolver's _paths.
The kernel loader prefers ``async_resolve`` when present; that is the only
path that can lazily activate such a module from its ``source_hint``.

The bundle resolver raises the KERNEL's ModuleNotFoundError
(amplifier_core.module_sources), which is NOT a subclass of the builtin --
the pre-existing tests above only ever used the builtin, which is how the
mismatch survived.
"""

def test_kernel_exception_is_not_the_builtin(self):
"""Pin the fact this fix exists for. If this ever changes, the tuple
catch below becomes redundant but stays harmless."""
assert not issubclass(CoreModuleNotFoundError, ModuleNotFoundError)

def test_sync_resolve_falls_back_on_kernel_exception(self, tmp_path: Path):
"""THE sync-path defect: the kernel class escaped ``except ModuleNotFoundError``
and the settings fallback never ran."""
module_dir = tmp_path / "m"
module_dir.mkdir()
bundle_resolver = MagicMock()
bundle_resolver.resolve.side_effect = CoreModuleNotFoundError("not prepared")
settings_resolver = MagicMock()
settings_resolver.resolve.return_value = FoundationFileSource(module_dir)

app = AppModuleResolver(bundle_resolver, settings_resolver=settings_resolver)
result = app.resolve("late-module", source_hint="git+https://x/y@main")

assert isinstance(result, FoundationFileSource)
settings_resolver.resolve.assert_called_once_with(
"late-module", "git+https://x/y@main"
)

@pytest.mark.asyncio
async def test_async_resolve_uses_bundle_lazy_activation(self, tmp_path: Path):
"""The primary path: delegate to the bundle resolver's async_resolve so
foundation activates the module from the hint (clone, deps, _paths)."""
module_dir = tmp_path / "m"
module_dir.mkdir()
bundle_resolver = MagicMock()
bundle_resolver.async_resolve = AsyncMock(
return_value=FoundationFileSource(module_dir)
)
settings_resolver = MagicMock()

app = AppModuleResolver(bundle_resolver, settings_resolver=settings_resolver)
result = await app.async_resolve(
"hooks-routing",
source_hint="git+https://github.com/microsoft/amplifier-bundle-routing-matrix@main#subdirectory=modules/hooks-routing",
)

assert isinstance(result, FoundationFileSource)
bundle_resolver.async_resolve.assert_awaited_once_with(
"hooks-routing",
source_hint="git+https://github.com/microsoft/amplifier-bundle-routing-matrix@main#subdirectory=modules/hooks-routing",
)
settings_resolver.resolve.assert_not_called()

@pytest.mark.asyncio
async def test_async_resolve_falls_back_to_settings_on_kernel_exception(
self, tmp_path: Path
):
module_dir = tmp_path / "m"
module_dir.mkdir()
bundle_resolver = MagicMock()
bundle_resolver.async_resolve = AsyncMock(
side_effect=CoreModuleNotFoundError("activation failed")
)
settings_resolver = MagicMock()
settings_resolver.resolve.return_value = FoundationFileSource(module_dir)

app = AppModuleResolver(bundle_resolver, settings_resolver=settings_resolver)
result = await app.async_resolve("late-module", source_hint="hint")

assert isinstance(result, FoundationFileSource)
settings_resolver.resolve.assert_called_once_with("late-module", "hint")

@pytest.mark.asyncio
async def test_async_resolve_without_bundle_async_uses_sync_bundle(
self, tmp_path: Path
):
"""A bundle resolver lacking async_resolve (older foundation) still works."""
module_dir = tmp_path / "m"
module_dir.mkdir()
bundle_resolver = MagicMock(spec=["resolve", "_paths"])
bundle_resolver.resolve.return_value = FoundationFileSource(module_dir)
bundle_resolver._paths = {}

app = AppModuleResolver(bundle_resolver)
result = await app.async_resolve("m", source_hint="hint")

assert isinstance(result, FoundationFileSource)
bundle_resolver.resolve.assert_called_once_with("m", "hint")

@pytest.mark.asyncio
async def test_async_resolve_raises_when_both_fail(self):
bundle_resolver = MagicMock()
bundle_resolver.async_resolve = AsyncMock(
side_effect=CoreModuleNotFoundError("nope")
)
bundle_resolver._paths = {"tool-bash": Path("/x")}
settings_resolver = MagicMock()
settings_resolver.resolve.side_effect = ModuleResolutionError("nope")

app = AppModuleResolver(bundle_resolver, settings_resolver=settings_resolver)
with pytest.raises(
ModuleNotFoundError, match="not found in bundle or user settings"
):
await app.async_resolve("ghost")

@pytest.mark.asyncio
async def test_async_resolve_profile_hint_alias(self, tmp_path: Path):
"""Backward compat: the deprecated profile_hint still feeds the hint."""
module_dir = tmp_path / "m"
module_dir.mkdir()
bundle_resolver = MagicMock()
bundle_resolver.async_resolve = AsyncMock(
return_value=FoundationFileSource(module_dir)
)
app = AppModuleResolver(bundle_resolver)
await app.async_resolve("m", profile_hint="legacy-hint")
bundle_resolver.async_resolve.assert_awaited_once_with(
"m", source_hint="legacy-hint"
)
Loading
Loading