diff --git a/amplifier_foundation/spawn_utils.py b/amplifier_foundation/spawn_utils.py index 84f0a27..86d5301 100644 --- a/amplifier_foundation/spawn_utils.py +++ b/amplifier_foundation/spawn_utils.py @@ -233,8 +233,17 @@ async def resolve_model_pattern( matched_models=[], ) - # Match pattern against available models - matched = fnmatch.filter(available_models, model_hint) + # Match pattern against available models: case-insensitive, OS-independent. + # Raw fnmatch.filter() uses os.path.normcase, which is case-sensitive on + # Linux/Mac and case-insensitive on Windows -- an OS-dependent + # inconsistency. Lowercasing both sides before comparing matches the + # canonical model-glob semantics used by the routing-matrix resolver + # (amplifier_module_hooks_routing.resolver) and the unified-llm-client + # reference implementation, so a pattern like "qwen3.6-*" deterministically + # matches "Qwen3.6-35B-A3B-..." on every platform. Original casing of the + # matched model name is preserved in the result. + lowered_hint = model_hint.lower() + matched = [m for m in available_models if fnmatch.fnmatch(m.lower(), lowered_hint)] if not matched: logger.warning( diff --git a/tests/test_bundle.py b/tests/test_bundle.py index a3e078b..cb1e422 100644 --- a/tests/test_bundle.py +++ b/tests/test_bundle.py @@ -105,6 +105,91 @@ def test_compose_instruction_replaced(self) -> None: result = base.compose(child) assert result.instruction == "Child instruction" + def test_compose_propagates_hooks_routing_config_to_child(self) -> None: + """BUG 3 regression: hooks-routing config (default_matrix, + custom_routing_dirs, overrides) injected into the parent/root + session's bundle must survive composition into a spawned child + bundle -- this is the mechanism PreparedBundle.spawn() relies on + (self.bundle.compose(child_bundle).to_mount_plan()) to make a + sub-session resolve model roles via the SAME user custom routing + matrix as its parent. + + This does not re-test amplifier_module_hooks_routing's own mount() + logic (covered in the routing-matrix repo) -- it verifies the + foundation-owned propagation mechanism that carries the hook's + config dict, unmodified, from parent bundle to child mount plan. + """ + parent_hooks_routing_config = { + "default_matrix": "ornith", + "custom_routing_dirs": ["/home/user/.amplifier/routing"], + "overrides": {"coding": {"description": "Coding override"}}, + } + parent = Bundle( + name="parent", + hooks=[ + {"module": "hooks-routing", "config": parent_hooks_routing_config}, + {"module": "hooks-logging"}, + ], + ) + # Child (agent) bundle declares no hooks-routing entry of its own -- + # the common case for a sub-agent bundle that doesn't override routing. + child = Bundle(name="child", hooks=[{"module": "tool-specific-hook"}]) + + composed = parent.compose(child) + mount_plan = composed.to_mount_plan() + + routing_entries = [ + h for h in mount_plan["hooks"] if h.get("module") == "hooks-routing" + ] + assert len(routing_entries) == 1, ( + f"hooks-routing must propagate to the child mount plan, got hooks: " + f"{mount_plan['hooks']}" + ) + assert routing_entries[0]["config"] == parent_hooks_routing_config, ( + "hooks-routing config (default_matrix/custom_routing_dirs/overrides) " + f"must reach the child unmodified, got: {routing_entries[0]['config']}" + ) + + def test_compose_child_hooks_routing_override_merges_not_replaces(self) -> None: + """When BOTH parent and child declare hooks-routing, the configs are + deep-merged (child wins on conflicts) rather than the child silently + replacing the parent's entire config -- so a child bundle overriding + just `overrides` does not accidentally drop the parent's + custom_routing_dirs (which is what makes a spawned sub-session able + to resolve a user's custom matrix at all).""" + parent = Bundle( + name="parent", + hooks=[ + { + "module": "hooks-routing", + "config": { + "default_matrix": "ornith", + "custom_routing_dirs": ["/home/user/.amplifier/routing"], + }, + } + ], + ) + child = Bundle( + name="child", + hooks=[ + { + "module": "hooks-routing", + "config": {"overrides": {"coding": {"description": "x"}}}, + } + ], + ) + + mount_plan = parent.compose(child).to_mount_plan() + cfg = next( + h["config"] for h in mount_plan["hooks"] if h["module"] == "hooks-routing" + ) + assert cfg["default_matrix"] == "ornith" + assert cfg["custom_routing_dirs"] == ["/home/user/.amplifier/routing"], ( + "Parent's custom_routing_dirs must survive even when the child " + f"bundle also declares hooks-routing config, got: {cfg}" + ) + assert cfg["overrides"] == {"coding": {"description": "x"}} + class TestBundleToMountPlan: """Tests for Bundle.to_mount_plan method.""" diff --git a/tests/test_spawn_utils.py b/tests/test_spawn_utils.py index bcf8476..96cc6ec 100644 --- a/tests/test_spawn_utils.py +++ b/tests/test_spawn_utils.py @@ -267,6 +267,58 @@ async def test_pattern_no_matches_returns_pattern(self) -> None: assert result.resolved_model == "claude-*" assert result.matched_models == [] + @pytest.mark.asyncio + async def test_pattern_matches_case_insensitively(self) -> None: + """Regression test: glob matching must be case-insensitive and + OS-independent. Raw fnmatch.filter() uses os.path.normcase, which is + case-sensitive on Linux/Mac and case-insensitive on Windows -- so a + mixed-case model id (e.g. real-world 'Qwen3.6-35B-A3B-UD-Q4_K_XL') + would silently fail to match a lowercase pattern ('qwen3.6-*') on + Linux/Mac while matching on Windows. Model glob matching must be + deterministic across platforms, and consistent with the routing-matrix + resolver's semantics (amplifier_module_hooks_routing.resolver). + """ + mock_provider = AsyncMock() + mock_provider.list_models = AsyncMock( + return_value=["Qwen3.6-35B-A3B-UD-Q4_K_XL"] + ) + + mock_coordinator = MagicMock() + mock_coordinator.get.return_value = {"provider-ornith": mock_provider} + + result = await resolve_model_pattern( + "qwen3.6-*", + "ornith", + mock_coordinator, + ) + + assert result.resolved_model == "Qwen3.6-35B-A3B-UD-Q4_K_XL", ( + f"Expected case-insensitive match to find the mixed-case model, " + f"got: {result.resolved_model!r}" + ) + assert result.matched_models == ["Qwen3.6-35B-A3B-UD-Q4_K_XL"] + + @pytest.mark.asyncio + async def test_uppercase_pattern_matches_lowercase_model(self) -> None: + """Symmetric case: an uppercase-leaning pattern must match a + lowercase model id -- proves the fix lowercases BOTH sides, not just + the model list.""" + mock_provider = AsyncMock() + mock_provider.list_models = AsyncMock( + return_value=["qwen3.6-35b-a3b-ud-q4_k_xl"] + ) + + mock_coordinator = MagicMock() + mock_coordinator.get.return_value = {"provider-ornith": mock_provider} + + result = await resolve_model_pattern( + "Qwen3.6-*", + "ornith", + mock_coordinator, + ) + + assert result.resolved_model == "qwen3.6-35b-a3b-ud-q4_k_xl" + class TestApplyProviderPreferencesWithResolution: """Tests for apply_provider_preferences_with_resolution function."""