fix(aux-routing): _read_main_provider string-form inference + credential transient-failure hardening

agent/auxiliary_client.py

@@ -911,6 +911,34 @@ def _read_main_provider() -> str:
             provider = model_cfg.get("provider", "")
             if isinstance(provider, str) and provider.strip():
                 return provider.strip().lower()
+        # ── Myah: infer provider from string-form model: ─────────────────────
+        # Myah's PATCH /api/v1/agent/config path writes model: as a bare string
+        # (e.g. "anthropic/claude-haiku-4-5-20251001") via set_config_value().
+        # The dict-form branch above is a no-op for such configs, leaving
+        # _resolve_auto Step 1 unable to determine the provider and falling
+        # through to the hardcoded OpenRouter fallback regardless of what model
+        # the user picked.  Mirror _read_main_model's string-form handling
+        # (auxiliary_client.py:889-890) and delegate to detect_provider_for_model
+        # — the same helper the interactive `hermes model` flow uses when writing
+        # dict-form config.  Lazy import avoids circular-import risk at boot.
+        elif isinstance(model_cfg, str) and model_cfg.strip():
+            model_str = model_cfg.strip()
+            # Fast path: vendor-prefixed slug like "anthropic/claude-haiku-..."
+            # where the prefix is a known provider id in _PROVIDER_MODELS.
+            if "/" in model_str:
+                from hermes_cli.models import _PROVIDER_MODELS
+                prefix = model_str.split("/", 1)[0].lower()
+                if prefix in _PROVIDER_MODELS:
+                    return prefix
+            # Fallback: catalog-based inference via the same helper the
+            # interactive `hermes model` flow uses to write dict-form config.
+            from hermes_cli.models import detect_provider_for_model
+            result = detect_provider_for_model(model_str, current_provider="")
+            if result is not None:
+                provider_id, _resolved_model = result
+                if isinstance(provider_id, str) and provider_id.strip():
+                    return provider_id.strip().lower()
+        # ─────────────────────────────────────────────────────────────────────
     except Exception:
         pass
     return ""

gateway/platforms/myah_management.py

@@ -1542,12 +1542,24 @@ async def handle_provider_models(request: web.Request) -> web.Response:
     return web.json_response([{"id": m, "name": m} for m in ids])
 
 
-async def _validate_api_key(catalog_entry: dict, api_key: str) -> bool:
-    """Validate the API key by hitting the provider's validation URL."""
+# ── Myah: credential validation with transient-failure handling ───────────────
+# Native Hermes CLI accepts keys optimistically (no validation URL hit). This
+# function adds early-feedback validation for the Myah onboarding UI, but must
+# not treat network transients as auth failures — that turns a DNS flake or a
+# rate-limit into a false "invalid key" rejection. Only explicit 401/403
+# responses (provider-side auth denial) should block a credential save.
+async def _validate_api_key(catalog_entry: dict, api_key: str) -> tuple[bool, str]:
+    """Validate the API key against the provider's validation URL.
+
+    Returns (accepted, reason) where accepted=True means the key should be
+    persisted. Only HTTP 401/403 (explicit auth denial) returns (False, ...).
+    Timeouts, 429, 5xx, and network errors return (True, 'optimistic accept ...')
+    so that transient infra issues do not permanently block valid credentials.
+    """
     validation = catalog_entry.get("validation") or {}
     url = validation.get("url")
     if not url:
-        return True  # optimistic accept when no validation URL configured
+        return True, "no validation URL configured"
     headers: dict = {}
     params = None
     method = validation.get("method", "GET")
@@ -1565,9 +1577,24 @@ async def _validate_api_key(catalog_entry: dict, api_key: str) -> bool:
             async with session.request(
                 method, url, headers=headers, params=params
             ) as r:
-                return r.status < 400
-    except Exception:
-        return False
+                if r.status in (401, 403):
+                    return False, f"auth denied by provider (HTTP {r.status})"
+                if r.status < 400:
+                    return True, "validated"
+                # 429 / 5xx / other — cannot confirm auth; accept optimistically
+                logger.warning(
+                    f"[myah] validation endpoint returned {r.status} for {url}; optimistic accept"
+                )
+                return True, f"optimistic accept (validation HTTP {r.status})"
+    except asyncio.TimeoutError:
+        logger.warning(f"[myah] validation endpoint timed out for {url}; optimistic accept")
+        return True, "optimistic accept (validation timeout)"
+    except Exception as exc:
+        logger.warning(
+            f"[myah] validation endpoint error for {url}: {exc}; optimistic accept"
+        )
+        return True, f"optimistic accept (validation error: {exc})"
+# ──────────────────────────────────────────────────────────────────────────────
 
 
 async def handle_connect_credential(request: web.Request) -> web.Response:
@@ -1592,11 +1619,13 @@ async def handle_connect_credential(request: web.Request) -> web.Response:
     if not entry:
         return web.json_response({"error": f"unknown provider: {provider_id}"}, status=404)
 
-    valid = await _validate_api_key(entry, api_key)
-    if not valid:
+    accepted, reason = await _validate_api_key(entry, api_key)
+    if not accepted:
         return web.json_response(
-            {"error": f"validation failed for {provider_id}"}, status=400
+            {"error": f"validation failed for {provider_id}: {reason}"}, status=400
         )
+    if "optimistic" in reason:
+        logger.info(f"[myah] credential connect {provider_id}: {reason}")
 
     write_type = entry.get("write_type", "env_var")
     key_last_four = api_key[-4:] if len(api_key) >= 4 else "****"

tests/agent/test_auxiliary_client.py

@@ -978,3 +978,204 @@ def test_jpeg_media_type_parsed(self):
         }]
         result = _convert_openai_images_to_anthropic(messages)
         assert result[0]["content"][0]["source"]["media_type"] == "image/jpeg"
+
+
+# ── Task 1: Failing regression tests for _read_main_provider string-form ────
+# These tests pin the missing string-form branch in _read_main_provider.
+# Tests 1, 2 (and only those) should fail against the unpatched code because
+# the function returns "" for any string-form model: value.
+# Tests 3-7 should already pass (dict-form + edge cases work today).
+# ────────────────────────────────────────────────────────────────────────────
+
+class TestReadMainProvider:
+    """Unit tests for _read_main_provider() covering string-form inference."""
+
+    def test_read_main_provider_infers_from_string_model_anthropic(self):
+        """String-form model: with anthropic slug must return 'anthropic'."""
+        from agent.auxiliary_client import _read_main_provider
+        with patch("hermes_cli.config.load_config",
+                   return_value={"model": "anthropic/claude-haiku-4-5-20251001"}):
+            result = _read_main_provider()
+        assert result == "anthropic"
+
+    def test_read_main_provider_infers_from_string_model_openrouter(self):
+        """String-form model: with openrouter-only slug must return 'openrouter'."""
+        from agent.auxiliary_client import _read_main_provider
+        # google/gemini-3-flash-preview is only in the openrouter catalog
+        with patch("hermes_cli.config.load_config",
+                   return_value={"model": "google/gemini-3-flash-preview"}):
+            result = _read_main_provider()
+        assert result == "openrouter"
+
+    def test_read_main_provider_dict_form_unchanged(self):
+        """Dict-form model: with explicit provider must return that provider unchanged."""
+        from agent.auxiliary_client import _read_main_provider
+        with patch("hermes_cli.config.load_config",
+                   return_value={"model": {"provider": "zai", "default": "glm-4.5-flash"}}):
+            result = _read_main_provider()
+        assert result == "zai"
+
+    def test_read_main_provider_dict_form_auto_provider_passes_through(self):
+        """Dict-form model: with provider='auto', function returns 'auto'.
+
+        _resolve_auto at :1292 already filters out 'auto' with the guard
+        `main_provider not in ("auto", "")` — so the string 'auto' reaching
+        _resolve_auto is harmless. This test pins the existing pass-through
+        behavior so it is not accidentally broken during the fix.
+        """
+        from agent.auxiliary_client import _read_main_provider
+        with patch("hermes_cli.config.load_config",
+                   return_value={"model": {"provider": "auto", "default": "openrouter/x"}}):
+            result = _read_main_provider()
+        assert result == "auto"
+
+    def test_read_main_provider_unknown_model_returns_empty(self):
+        """String-form model: with unrecognised id must return '' (no crash)."""
+        from agent.auxiliary_client import _read_main_provider
+        with patch("hermes_cli.config.load_config",
+                   return_value={"model": "gibberish/nonexistent-model-xyz"}):
+            result = _read_main_provider()
+        assert result == ""
+
+    def test_read_main_provider_empty_string_returns_empty(self):
+        """Empty string model: must short-circuit and return ''."""
+        from agent.auxiliary_client import _read_main_provider
+        with patch("hermes_cli.config.load_config", return_value={"model": ""}):
+            result = _read_main_provider()
+        assert result == ""
+
+    def test_read_main_provider_missing_model_key_returns_empty(self):
+        """No model key in config must return ''."""
+        from agent.auxiliary_client import _read_main_provider
+        with patch("hermes_cli.config.load_config", return_value={}):
+            result = _read_main_provider()
+        assert result == ""
+
+
+# ── Task 3: Edge-case tests ───────────────────────────────────────────────────
+# Additional corner-case coverage for _read_main_provider after the fix lands.
+# ─────────────────────────────────────────────────────────────────────────────
+
+class TestReadMainProviderEdgeCases:
+    """Edge-case and regression tests for _read_main_provider()."""
+
+    def test_read_main_provider_vendor_prefix_uses_detect_as_ground_truth(self):
+        """Vendor-prefixed slug uses detect_provider_for_model as ground truth.
+
+        Test is intentionally resilient to upstream catalog changes: it calls
+        detect_provider_for_model itself to get the expected value rather than
+        hard-coding it, so the test keeps passing even if catalog entries shift.
+        """
+        from agent.auxiliary_client import _read_main_provider
+        from hermes_cli.models import detect_provider_for_model
+        model = "google/gemini-3-flash-preview"
+        expected_result = detect_provider_for_model(model, "")
+        # The fast path hits: 'google' is NOT a known provider key, so falls
+        # through to detect_provider_for_model which returns ('openrouter', ...).
+        with patch("hermes_cli.config.load_config", return_value={"model": model}):
+            result = _read_main_provider()
+        if expected_result is not None:
+            assert result == expected_result[0].strip().lower()
+        else:
+            assert result == ""
+
+    def test_read_main_provider_dict_with_no_provider_key_returns_empty(self):
+        """Dict-form model: with no provider key returns '' (intentionally unchanged).
+
+        A dict {default: "..."} with no explicit 'provider' key is not extended
+        by this fix — that's a separate bug surface. This test documents the
+        intentional scope limit so future refactors are aware.
+        """
+        from agent.auxiliary_client import _read_main_provider
+        with patch("hermes_cli.config.load_config",
+                   return_value={"model": {"default": "anthropic/claude-haiku-4-5-20251001"}}):
+            result = _read_main_provider()
+        assert result == ""
+
+    def test_read_main_provider_swallows_load_config_exception(self):
+        """load_config raising must not propagate — returns '' safely."""
+        from agent.auxiliary_client import _read_main_provider
+        with patch("hermes_cli.config.load_config", side_effect=RuntimeError("disk error")):
+            result = _read_main_provider()
+        assert result == ""
+
+    def test_read_main_provider_known_prefix_matches_provider(self):
+        """'anthropic/...' string form takes the fast-path and returns 'anthropic'."""
+        from agent.auxiliary_client import _read_main_provider
+        with patch("hermes_cli.config.load_config",
+                   return_value={"model": "anthropic/claude-sonnet-4-6"}):
+            result = _read_main_provider()
+        assert result == "anthropic"
+
+    def test_read_main_provider_unknown_prefix_falls_through_to_detect(self):
+        """Unknown vendor prefix falls through to detect_provider_for_model."""
+        from agent.auxiliary_client import _read_main_provider
+        # 'totally-unknown-provider/some-model' — prefix not in _PROVIDER_MODELS,
+        # detect_provider_for_model also can't match it → returns ''.
+        with patch("hermes_cli.config.load_config",
+                   return_value={"model": "totally-unknown-provider/some-model"}):
+            result = _read_main_provider()
+        assert result == ""
+
+
+# ── Appendix Task F: Honcho-aux-isolation invariant tests ────────────────────
+# Regression fence: aux calls must NEVER interact with Honcho memory.
+# Current code is clean (verified 2026-04-22 via grep); these tests ensure
+# future refactors can't silently re-couple the aux path to user memory.
+# ─────────────────────────────────────────────────────────────────────────────
+
+class TestHonchoAuxIsolation:
+    """Invariant: aux LLM calls must never read from or write to Honcho memory."""
+
+    def test_auxiliary_client_has_no_honcho_dependencies(self):
+        """auxiliary_client.py must not import or reference Honcho or MemoryManager.
+
+        Rationale: aux tasks (title, follow-ups, compression, vision,
+        session_search) use a separate, Honcho-free LLM client. Only the
+        main agent loop is authorised to touch Honcho. This test fails
+        immediately if someone accidentally re-introduces aux→Honcho coupling
+        via an import, direct reference, or inline usage.
+        """
+        import agent.auxiliary_client
+        import inspect
+
+        source = inspect.getsource(agent.auxiliary_client)
+        forbidden = [
+            "honcho", "HonchoClient", "memory_manager",
+            "MemoryManager", "peer_card",
+        ]
+        found = [term for term in forbidden if term.lower() in source.lower()]
+        assert not found, (
+            "auxiliary_client.py must not reference Honcho or memory_manager. "
+            f"Found: {found}. Aux calls must never interact with user memory. "
+            "Only the main agent loop may access Honcho."
+        )
+
+    def test_myah_gateway_aux_endpoint_has_no_honcho_dependencies(self):
+        """The _handle_aux_endpoint body in myah.py must not reference Honcho.
+
+        The Myah HTTP /myah/v1/aux/{task} endpoint delegates to
+        auxiliary_client.call_llm — it must not inject any Honcho context.
+        Other parts of myah.py that set up Honcho for the main agent are fine.
+        """
+        import re
+        import gateway.platforms.myah
+        import inspect
+
+        source = inspect.getsource(gateway.platforms.myah)
+        # Locate _handle_aux_endpoint function body up to the next top-level
+        # definition or end of source.
+        m = re.search(
+            r"def\s+_handle_aux_endpoint.*?(?=\n(?:async\s+)?def\s|\nclass\s|\Z)",
+            source,
+            re.DOTALL,
+        )
+        assert m, "Could not locate _handle_aux_endpoint body in gateway.platforms.myah"
+        aux_body = m.group(0)
+
+        forbidden = ["honcho", "memory_manager", "peer_card"]
+        found = [term for term in forbidden if term.lower() in aux_body.lower()]
+        assert not found, (
+            f"_handle_aux_endpoint body must not reference Honcho. Found: {found}. "
+            "The aux HTTP endpoint must be a pure LLM call with no memory context."
+        )