Skip to content
Closed
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -69,5 +69,6 @@ next-steps.md
/amplifier-app-cli/

# Working folders
.next/
ai_working/tmp
tests/recipes/DECISIONS.md
9 changes: 8 additions & 1 deletion amplifier_app_cli/lib/bundle_loader/prepare.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ async def load_and_prepare_bundle(
source_overrides: dict[str, str] | None = None,
progress_callback: Callable[[str, str], None] | None = None,
bundle_source_overrides: dict[str, str] | None = None,
required_behaviors: set[str] | None = None,
) -> PreparedBundle:
"""Load bundle by name or URI and prepare it for execution.

Expand Down Expand Up @@ -83,6 +84,10 @@ async def load_and_prepare_bundle(
Keys are matched as substrings of include URIs. If matched, the override
URI is used instead.
Example: {"amplifier-bundle-superpowers": "/local/path"}
required_behaviors: Optional subset of ``compose_behaviors`` whose
load or composition failures must propagate. Other behavior
failures remain warnings for backward compatibility with optional
policies such as notifications.

Returns:
PreparedBundle ready for create_session().
Expand Down Expand Up @@ -168,8 +173,10 @@ async def load_and_prepare_bundle(
f"Composed behavior '{behavior_bundle.name}' onto '{bundle.name}'"
)
except Exception as e:
if required_behaviors and behavior_uri in required_behaviors:
raise
logger.warning(f"Failed to compose behavior '{behavior_uri}': {e}")
# Continue without this behavior - notifications are optional
# Continue without optional behaviors such as notifications.

# 3b. Load agent metadata BEFORE prepare so the agent's declared modules
# (tools/providers/hooks with `source:` URIs in their .md frontmatter) are
Expand Down
33 changes: 28 additions & 5 deletions amplifier_app_cli/runtime/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,15 @@ def _on_progress(action: str, detail: str) -> None:
_build_notification_behaviors(app_settings.get_notification_flags())
)

# Routing matrix behavior. Resolve routing once so the same active
# configuration drives both pre-prepare composition and the
# post-prepare hooks-routing config merge below.
routing_config = app_settings.get_routing_config()
routing_behaviors = _build_routing_behaviors(routing_config)
compose_behaviors.extend(routing_behaviors)

# Add app bundles (user-configured bundles that are always composed)
# App bundles are explicit user configuration, composed AFTER notification behaviors
# App bundles are explicit user configuration, composed AFTER app policy behaviors
app_bundles = app_settings.get_app_bundles()
if app_bundles:
compose_behaviors = compose_behaviors + app_bundles
Expand Down Expand Up @@ -133,6 +140,7 @@ def _on_progress(action: str, detail: str) -> None:
bundle_name,
discovery,
compose_behaviors=compose_behaviors if compose_behaviors else None,
required_behaviors=set(routing_behaviors) if routing_behaviors else None,
source_overrides=combined_sources if combined_sources else None,
bundle_source_overrides=bundle_sources if bundle_sources else None,
progress_callback=_on_progress if status else None,
Expand Down Expand Up @@ -231,7 +239,6 @@ def _on_progress(action: str, detail: str) -> None:
hook_overrides = app_settings.get_notification_hook_overrides()

# Routing matrix config injection
routing_config = app_settings.get_routing_config()
if routing_config:
routing_hook_override: dict[str, Any] = {
"module": "hooks-routing",
Expand Down Expand Up @@ -314,9 +321,9 @@ def _on_progress(action: str, detail: str) -> None:
prepared, bundle_config, sync_tools=bool(bundle_config.get("tools"))
)

# Note: Notification hooks are now composed via compose_behaviors parameter
# to load_and_prepare_bundle(), so they get properly installed during prepare().
# The behavior bundles handle root-session-only logic internally via parent_id check.
# Note: Notification and routing hooks are composed via compose_behaviors
# before load_and_prepare_bundle() prepares their modules.
# Notification behaviors handle root-session-only logic internally via parent_id.

return bundle_config, prepared

Expand Down Expand Up @@ -884,6 +891,22 @@ def _build_modes_behaviors() -> list[str]:
]


def _build_routing_behaviors(routing_config: dict[str, Any]) -> list[str]:
"""Return the routing behavior URI when routing configuration is active.

Routing is an app-level policy. Its behavior must be composed before
preparation so ``hooks-routing`` and its module source are available to
the prepared bundle. The resolved config is also reused after preparation
to apply matrix selection and routing overrides.
"""
if not routing_config:
return []

return [
"git+https://github.com/microsoft/amplifier-bundle-routing-matrix@main#subdirectory=behaviors/routing.yaml",
]


def _build_notification_behaviors(flags: NotificationFlags) -> list[str]:
"""Build list of notification behavior URIs based on resolved flags.

Expand Down
64 changes: 64 additions & 0 deletions tests/lib/bundle_loader/test_prepare.py
Original file line number Diff line number Diff line change
Expand Up @@ -241,3 +241,67 @@ async def test_no_bundle_overrides_skips_resolver(self):

# set_include_source_resolver was NOT called
mock_registry.set_include_source_resolver.assert_not_called()


class TestLoadAndPrepareBundleRequiredBehaviors:
"""Required behavior failures must not be hidden by the loader."""

@pytest.mark.asyncio
async def test_required_behavior_load_failure_propagates(self):
"""A routing behavior load error aborts before bundle preparation."""
from amplifier_app_cli.lib.bundle_loader.prepare import load_and_prepare_bundle

routing_uri = (
"git+https://github.com/microsoft/amplifier-bundle-routing-matrix@main"
"#subdirectory=behaviors/routing.yaml"
)
mock_discovery = MagicMock()
mock_discovery.find.return_value = "file:///path/to/bundle.yaml"

mock_bundle = MagicMock()
mock_bundle.prepare = AsyncMock()
load_error = RuntimeError("hooks-routing module unavailable")

with (
patch(
"amplifier_app_cli.lib.bundle_loader.prepare.load_bundle",
new_callable=AsyncMock,
side_effect=[mock_bundle, load_error],
),
pytest.raises(RuntimeError, match="hooks-routing module unavailable"),
):
await load_and_prepare_bundle(
"my-bundle",
mock_discovery,
compose_behaviors=[routing_uri],
required_behaviors={routing_uri},
)

mock_bundle.prepare.assert_not_awaited()

@pytest.mark.asyncio
async def test_optional_behavior_load_failure_is_still_ignored(self):
"""Unrelated optional behavior failures preserve existing semantics."""
from amplifier_app_cli.lib.bundle_loader.prepare import load_and_prepare_bundle

optional_uri = "file:///optional-notifications.yaml"
mock_discovery = MagicMock()
mock_discovery.find.return_value = "file:///path/to/bundle.yaml"

mock_bundle = MagicMock()
mock_prepared = MagicMock()
mock_bundle.prepare = AsyncMock(return_value=mock_prepared)

with patch(
"amplifier_app_cli.lib.bundle_loader.prepare.load_bundle",
new_callable=AsyncMock,
side_effect=[mock_bundle, RuntimeError("optional behavior unavailable")],
):
result = await load_and_prepare_bundle(
"my-bundle",
mock_discovery,
compose_behaviors=[optional_uri],
)

assert result is mock_prepared
mock_bundle.prepare.assert_awaited_once()
102 changes: 100 additions & 2 deletions tests/test_general_config_overrides.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,19 @@
"""

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

import pytest
from unittest.mock import AsyncMock, MagicMock, patch

from amplifier_app_cli.lib.merge_utils import deep_merge
from amplifier_app_cli.lib.settings import NotificationFlags
from amplifier_app_cli.runtime.config import (
_apply_hook_overrides,
_apply_provider_overrides,
_apply_tool_overrides,
resolve_bundle_config,
)


# ═══════════════════════════════════════════════════════════════════════════
# PART 1: Direct logic tests — exercise the exact code path added in the fix
# ═══════════════════════════════════════════════════════════════════════════
Expand Down Expand Up @@ -318,6 +319,11 @@ def _make_app_settings(config_overrides=None, **kwargs):
"hook_overrides", []
)
settings.get_routing_config.return_value = kwargs.get("routing_config", None)
settings.get_notification_flags.return_value = NotificationFlags(
desktop_enabled=False,
push_enabled=False,
)
settings.get_app_bundles.return_value = []
settings.get_source_overrides.return_value = {}
settings.get_module_sources.return_value = {}
settings.get_bundle_sources.return_value = {}
Expand All @@ -331,6 +337,98 @@ class TestFullPipelineIntegration:
at their SOURCE modules since they're imported inside the function body.
"""

@pytest.mark.asyncio
async def test_active_routing_composed_before_prepare(self):
"""Active routing sources its behavior through the prepare workflow."""
mock_prepared = MagicMock()
mock_prepared.mount_plan = {"hooks": []}
mock_prepared.bundle.load_agent_metadata = MagicMock()
settings = _make_app_settings(
routing_config={
"matrix": "balanced",
"overrides": {"coding": "quality"},
}
)
prepare = AsyncMock(return_value=mock_prepared)

with (
patch(
"amplifier_app_cli.lib.bundle_loader.prepare.load_and_prepare_bundle",
prepare,
),
patch("amplifier_app_cli.paths.get_bundle_search_paths", return_value=[]),
patch("amplifier_app_cli.lib.bundle_loader.AppBundleDiscovery"),
):
result, _ = await resolve_bundle_config(
bundle_name="test", app_settings=settings
)

compose_behaviors = prepare.await_args_list[0].kwargs["compose_behaviors"]
required_behaviors = prepare.await_args_list[0].kwargs["required_behaviors"]
assert (
"git+https://github.com/microsoft/amplifier-bundle-routing-matrix@main"
"#subdirectory=behaviors/routing.yaml"
) in compose_behaviors
assert required_behaviors <= set(compose_behaviors)
assert any(
"amplifier-bundle-routing-matrix" in uri for uri in required_behaviors
)
assert result["hooks"][0]["config"]["default_matrix"] == "balanced"
assert result["hooks"][0]["config"]["overrides"] == {"coding": "quality"}
settings.get_routing_config.assert_called_once_with()

@pytest.mark.asyncio
async def test_inactive_routing_not_composed_before_prepare(self):
"""Absent routing config leaves the routing behavior out of composition."""
mock_prepared = MagicMock()
mock_prepared.mount_plan = {"hooks": []}
mock_prepared.bundle.load_agent_metadata = MagicMock()
settings = _make_app_settings(routing_config={})
prepare = AsyncMock(return_value=mock_prepared)

with (
patch(
"amplifier_app_cli.lib.bundle_loader.prepare.load_and_prepare_bundle",
prepare,
),
patch("amplifier_app_cli.paths.get_bundle_search_paths", return_value=[]),
patch("amplifier_app_cli.lib.bundle_loader.AppBundleDiscovery"),
):
result, _ = await resolve_bundle_config(
bundle_name="test", app_settings=settings
)

compose_behaviors = prepare.await_args_list[0].kwargs["compose_behaviors"]
required_behaviors = prepare.await_args_list[0].kwargs["required_behaviors"]
assert all(
"amplifier-bundle-routing-matrix" not in uri for uri in compose_behaviors
)
assert required_behaviors is None
assert result["hooks"] == []
settings.get_routing_config.assert_called_once_with()

@pytest.mark.asyncio
async def test_routing_preparation_error_propagates(self):
"""Errors from preparation remain fatal when routing is active."""
settings = _make_app_settings(routing_config={"matrix": "balanced"})
prepare = AsyncMock(side_effect=RuntimeError("hooks-routing unavailable"))

with (
patch(
"amplifier_app_cli.lib.bundle_loader.prepare.load_and_prepare_bundle",
prepare,
),
patch("amplifier_app_cli.paths.get_bundle_search_paths", return_value=[]),
patch("amplifier_app_cli.lib.bundle_loader.AppBundleDiscovery"),
pytest.raises(RuntimeError, match="hooks-routing unavailable"),
):
await resolve_bundle_config(bundle_name="test", app_settings=settings)

compose_behaviors = prepare.await_args_list[0].kwargs["compose_behaviors"]
assert any(
"amplifier-bundle-routing-matrix" in uri for uri in compose_behaviors
)

@pytest.mark.asyncio
async def test_hook_override_flows_through_full_pipeline(self):
"""Config override for a hook reaches final bundle_config."""
Expand Down