From e4e38ae8f0f397a1824c241dbd7cd9552a4de3ae Mon Sep 17 00:00:00 2001 From: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> Date: Mon, 7 Sep 2026 13:23:32 -0700 Subject: [PATCH] fix(resolver): canonical family name exhausts BOTH match steps before any alias `provider: openai` resolved to the ChatGPT subscription on a host with six provider-openai instances configured, the best at priority 2. Cause: find_provider_by_type ran step 1 (match by key) for EVERY accepted name before running step 2 (match by module type) for any. The openai instances are keyed by instance id (sol, terra, luna, ...), so `openai` missed by key; `openai-chatgpt` -- one instance, keyed by its bare type -- HIT by key and returned, before step 2 ever resolved `openai` through the mount plan. "Canonical first within each step" is not "canonical first". Fix: each accepted name gets both steps before the next name is tried. Verified against the reporting host's real settings.yaml: BEFORE provider: openai -> openai-chatgpt (provider-openai-chatgpt, priority 17) AFTER provider: openai -> terra (provider-openai, priority 2) Two tests: the exact production shape (instance-id-keyed canonicals + a bare-type-keyed alias -> canonical wins at best priority), and the case the alias exists for (no canonical anywhere -> alias still resolves). 601 -> 603. Generated with Amplifier Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- .../resolver.py | 40 ++++++++------ modules/hooks-routing/tests/test_resolver.py | 54 +++++++++++++++++++ 2 files changed, 78 insertions(+), 16 deletions(-) diff --git a/modules/hooks-routing/amplifier_module_hooks_routing/resolver.py b/modules/hooks-routing/amplifier_module_hooks_routing/resolver.py index 30793b9..92d82cc 100644 --- a/modules/hooks-routing/amplifier_module_hooks_routing/resolver.py +++ b/modules/hooks-routing/amplifier_module_hooks_routing/resolver.py @@ -191,9 +191,10 @@ def find_provider_by_type( Returns: ``(module_id, provider_instance)`` or ``None``. - Matching strategy (each step is run for the canonical ``type_name`` - FIRST and then for each of its :data:`PROVIDER_FAMILY_ALIASES`, so the - canonical backend always wins when more than one is mounted): + Matching strategy (BOTH steps are run for the canonical ``type_name`` + before EITHER is run for any of its :data:`PROVIDER_FAMILY_ALIASES`, so + the canonical backend always wins when more than one is mounted -- however + its instances happen to be keyed): 1. Exact key, "provider-" prefix stripped, or "provider-" prefix added — covers the single-instance case and any instance explicitly keyed by the bare type. @@ -215,9 +216,21 @@ def find_provider_by_type( convention used elsewhere in the ecosystem. """ accepted = _type_names_for(type_name) + provider_specs = _get_provider_specs(coordinator) - # Step 1, one full scan per accepted name so the canonical type beats an - # alias regardless of mount order. + # Each accepted name gets BOTH steps before the next name is tried. The + # canonical name must be exhausted -- by key AND by module type -- before + # any alias is considered, or the alias wins on a technicality. + # + # That technicality bit on 2026-09-07: a host with SIX provider-openai + # instances (sol, terra, luna, ...) all keyed by instance id, plus one + # provider-openai-chatgpt instance keyed by its bare type. The earlier + # shape ran step 1 for every name first: `openai` missed by key (no + # instance is keyed "openai"), then `openai-chatgpt` HIT by key -- and + # returned, before step 2 ever resolved `openai` through the mount plan. + # The explorer ran on the ChatGPT subscription while the API key sat + # unused at priority 2. Canonical-first has to mean "canonical through + # every avenue first", not "canonical first within each avenue". for wanted in accepted: for name, provider in providers.items(): if wanted in ( @@ -226,17 +239,12 @@ def find_provider_by_type( f"provider-{wanted}", ): return (name, provider) - - provider_specs = _get_provider_specs(coordinator) - if not provider_specs: - return None - - # Step 2, likewise: resolve by module type for the canonical name first, - # and only fall through to an alias when no canonical instance exists. - for wanted in accepted: - found = _resolve_by_module_type(providers, provider_specs, wanted, model_pattern) - if found is not None: - return found + if provider_specs: + found = _resolve_by_module_type( + providers, provider_specs, wanted, model_pattern + ) + if found is not None: + return found return None diff --git a/modules/hooks-routing/tests/test_resolver.py b/modules/hooks-routing/tests/test_resolver.py index acb9a21..74e55e9 100644 --- a/modules/hooks-routing/tests/test_resolver.py +++ b/modules/hooks-routing/tests/test_resolver.py @@ -1549,3 +1549,57 @@ def test_module_type_fallback_prefers_canonical_over_alias(self) -> None: ) result = find_provider_by_type(providers, "openai", coordinator) assert result == ("terra", terra) + + def test_canonical_by_module_type_beats_alias_by_key(self) -> None: + """THE production shape that shipped wrong on 2026-09-07. + + Six provider-openai instances keyed by INSTANCE ID (sol, terra, luna, + ...) -- so nothing is keyed "openai" -- plus one provider-openai-chatgpt + instance keyed by its BARE TYPE. The old per-step ordering matched + `openai-chatgpt` by key before it ever tried `openai` by module type, + and the explorer ran on the ChatGPT subscription while the API key sat + unused at priority 2. Canonical must be exhausted through BOTH + avenues before any alias is considered. + """ + instances = { + "sol": MagicMock(), + "terra": MagicMock(), + "luna": MagicMock(), + "openai-chatgpt": MagicMock(), # keyed by bare type -- the trap + } + coordinator = _make_coordinator_with_provider_specs( + [ + {"module": "provider-openai", "id": "sol", "config": {"priority": 3}}, + {"module": "provider-openai", "id": "terra", "config": {"priority": 2}}, + {"module": "provider-openai", "id": "luna", "config": {"priority": 14}}, + { + "module": "provider-openai-chatgpt", + "id": "openai-chatgpt", + "config": {"priority": 17}, + }, + ] + ) + result = find_provider_by_type(instances, "openai", coordinator) + assert result is not None + assert result[0] == "terra", ( + f"resolved to {result[0]!r}; the canonical openai instance with the " + "best priority must win over an alias that merely happens to be " + "keyed by its bare type" + ) + + def test_alias_still_wins_when_no_canonical_instance_exists_anywhere(self) -> None: + """The reorder must not break the case the alias exists for.""" + instances = {"chatgpt-main": MagicMock()} + coordinator = _make_coordinator_with_provider_specs( + [ + { + "module": "provider-openai-chatgpt", + "id": "chatgpt-main", + "config": {"priority": 5}, + } + ] + ) + assert find_provider_by_type(instances, "openai", coordinator) == ( + "chatgpt-main", + instances["chatgpt-main"], + )