ACP authenticate() method_id validation against provider

Fixes on record

Merged to main. A stamp counts the issues a fix is linked to.

• #13468 refactor(acp): validate method_id against advertised provider in authenticate()

  merged to main

  files: acp_adapter/server.py, hermes_cli/models.py, tests/acp/test_server.py, tests/hermes_cli/test_models.py

  Show the diff

  • acp_adapter/server.py

    @@ -51,7 +51,7 @@
     except ImportError:
         from acp.schema import AuthMethod as AuthMethodAgent  # type: ignore[attr-defined]
     
    -from acp_adapter.auth import detect_provider, has_provider
    +from acp_adapter.auth import detect_provider
     from acp_adapter.events import (
         make_message_cb,
         make_step_cb,
    @@ -351,9 +351,18 @@ async def initialize(
             )
     
         async def authenticate(self, method_id: str, **kwargs: Any) -> AuthenticateResponse | None:
    -        if has_provider():
    -            return AuthenticateResponse()
    -        return None
    +        # Only accept authenticate() calls whose method_id matches the
    +        # provider we advertised in initialize(). Without this check,
    +        # authenticate() would acknowledge any method_id as long as the
    +        # server has provider credentials configured — harmless under
    +        # Hermes' threat model (ACP is stdio-only, local-trust), but poor
    +        # API hygiene and confusing if ACP ever grows multi-method auth.
    +        provider = detect_provider()
    +        if not provider:
    +            return None
    +        if not isinstance(method_id, str) or method_id.strip().lower() != provider:
    +            return None
    +        return AuthenticateResponse()
     
         # ---- Session management -------------------------------------------------
     

  • hermes_cli/models.py

    @@ -670,6 +670,31 @@ def _openrouter_model_is_free(pricing: Any) -> bool:
             return False
     
     
    +def _openrouter_model_supports_tools(item: Any) -> bool:
    +    """Return True when the model's ``supported_parameters`` advertise tool calling.
    +
    +    hermes-agent is tool-calling-first — every provider path assumes the model
    +    can invoke tools. Models that don't advertise ``tools`` in their
    +    ``supported_parameters`` (e.g. image-only or completion-only models) cannot
    +    be driven by the agent loop and would fail at the first tool call.
    +
    +    **Permissive when the field is missing.** Some OpenRouter-compatible gateways
    +    (Nous Portal, private mirrors, older catalog snapshots) don't populate
    +    ``supported_parameters`` at all. Treat that as "unknown capability → allow"
    +    so the picker doesn't silently empty for those users. Only hide models
    +    whose ``supported_parameters`` is an explicit list that omits ``tools``.
    +
    +    Ported from Kilo-Org/kilocode#9068.
    +    """
    +    if not isinstance(item, dict):
    +        return True
    +    params = item.get("supported_parameters")
    +    if not isinstance(params, list):
    +        # Field absent / malformed / None — be permissive.
    +        return True
    +    return "tools" in params
    +
    +
     def fetch_openrouter_models(
         timeout: float = 8.0,
         *,
    @@ -712,6 +737,11 @@ def fetch_openrouter_models(
             live_item = live_by_id.get(preferred_id)
             if live_item is None:
                 continue
    +        # Hide models that don't advertise tool-calling support — hermes-agent
    +        # requires it and surfacing them leads to immediate runtime failures
    +        # when the user selects them. Ported from Kilo-Org/kilocode#9068.
    +        if not _openrouter_model_supports_tools(live_item):
    +            continue
             desc = "free" if _openrouter_model_is_free(live_item.get("pricing")) else ""
             curated.append((preferred_id, desc))
     

  • tests/acp/test_server.py

    @@ -95,19 +95,37 @@ async def test_initialize_capabilities_wire_format(self, agent):
     
     class TestAuthenticate:
         @pytest.mark.asyncio
    -    async def test_authenticate_with_provider_configured(self, agent, monkeypatch):
    +    async def test_authenticate_with_matching_method_id(self, agent, monkeypatch):
             monkeypatch.setattr(
    -            "acp_adapter.server.has_provider",
    -            lambda: True,
    +            "acp_adapter.server.detect_provider",
    +            lambda: "openrouter",
             )
             resp = await agent.authenticate(method_id="openrouter")
             assert isinstance(resp, AuthenticateResponse)
     
    +    @pytest.mark.asyncio
    +    async def test_authenticate_is_case_insensitive(self, agent, monkeypatch):
    +        monkeypatch.setattr(
    +            "acp_adapter.server.detect_provider",
    +            lambda: "openrouter",
    +        )
    +        resp = await agent.authenticate(method_id="OpenRouter")
    +        assert isinstance(resp, AuthenticateResponse)
    +
    +    @pytest.mark.asyncio
    +    async def test_authenticate_rejects_mismatched_method_id(self, agent, monkeypatch):
    +        monkeypatch.setattr(
    +            "acp_adapter.server.detect_provider",
    +            lambda: "openrouter",
    +        )
    +        resp = await agent.authenticate(method_id="totally-invalid-method")
    +        assert resp is None
    +
         @pytest.mark.asyncio
         async def test_authenticate_without_provider(self, agent, monkeypatch):
             monkeypatch.setattr(
    -            "acp_adapter.server.has_provider",
    -            lambda: False,
    +            "acp_adapter.server.detect_provider",
    +            lambda: None,
             )
             resp = await agent.authenticate(method_id="openrouter")
             assert resp is None

  • tests/hermes_cli/test_models.py

    @@ -88,6 +88,131 @@ def test_falls_back_to_static_snapshot_on_fetch_failure(self, monkeypatch):
     
             assert models == OPENROUTER_MODELS
     
    +    def test_filters_out_models_without_tool_support(self, monkeypatch):
    +        """Models whose supported_parameters omits 'tools' must not appear in the picker.
    +
    +        hermes-agent is tool-calling-first — surfacing a non-tool model leads to
    +        immediate runtime failures when the user selects it. Ported from
    +        Kilo-Org/kilocode#9068.
    +        """
    +        class _Resp:
    +            def __enter__(self):
    +                return self
    +
    +            def __exit__(self, exc_type, exc, tb):
    +                return False
    +
    +            def read(self):
    +                # opus-4.6 advertises tools → kept
    +                # nano-image has explicit supported_parameters that OMITS tools → dropped
    +                # qwen3.6-plus advertises tools → kept
    +                return (
    +                    b'{"data":['
    +                    b'{"id":"anthropic/claude-opus-4.6","pricing":{"prompt":"0.000015","completion":"0.000075"},'
    +                    b'"supported_parameters":["temperature","tools","tool_choice"]},'
    +                    b'{"id":"google/gemini-3-pro-image-preview","pricing":{"prompt":"0.00001","completion":"0.00003"},'
    +                    b'"supported_parameters":["temperature","response_format"]},'
    +                    b'{"id":"qwen/qwen3.6-plus","pricing":{"prompt":"0.000000325","completion":"0.00000195"},'
    +                    b'"supported_parameters":["tools","temperature"]}'
    +                    b']}'
    +                )
    +
    +        # Include the image-only id in the curated list so it has a chance to be surfaced.
    +        monkeypatch.setattr(
    +            _models_mod,
    +            "OPENROUTER_MODELS",
    +            [
    +                ("anthropic/claude-opus-4.6", ""),
    +                ("google/gemini-3-pro-image-preview", ""),
    +                ("qwen/qwen3.6-plus", ""),
    +            ],
    +        )
    +        monkeypatch.setattr(_models_mod, "_openrouter_catalog_cache", None)
    +        with patch("hermes_cli.models.urllib.request.urlopen", return_value=_Resp()):
    +            models = fetch_openrouter_models(force_refresh=True)
    +
    +        ids = [mid for mid, _ in models]
    +        assert "anthropic/claude-opus-4.6" in ids
    +        assert "qwen/qwen3.6-plus" in ids
    +        # Image-only model advertised supported_parameters WITHOUT tools → must be dropped.
    +        assert "google/gemini-3-pro-image-preview" not in ids
    +
    +    def test_permissive_when_supported_parameters_missing(self, monkeypatch):
    +        """Models missing the supported_parameters field keep appearing in the picker.
    +
    +        Some OpenRouter-compatible gateways (Nous Portal, private mirrors, older
    +        catalog snapshots) don't populate supported_parameters. Treating missing
    +        as 'unknown → allow' prevents the picker from silently emptying on
    +        those gateways.
    +        """
    +        class _Resp:
    +            def __enter__(self):
    +                return self
    +
    +            def __exit__(self, exc_type, exc, tb):
    +                return False
    +
    +            def read(self):
    +                # No supported_parameters field at all on either entry.
    +                return (
    +                    b'{"data":['
    +                    b'{"id":"anthropic/claude-opus-4.6","pricing":{"prompt":"0.000015","completion":"0.000075"}},'
    +                    b'{"id":"qwen/qwen3.6-plus","pricing":{"prompt":"0.000000325","completion":"0.00000195"}}'
    +                    b']}'
    +                )
    +
    +        monkeypatch.setattr(_models_mod, "_openrouter_catalog_cache", None)
    +        with patch("hermes_cli.models.urllib.request.urlopen", return_value=_Resp()):
    +            models = fetch_openrouter_models(force_refresh=True)
    +
    +        ids = [mid for mid, _ in models]
    +        assert "anthropic/claude-opus-4.6" in ids
    +        assert "qwen/qwen3.6-plus" in ids
    +
    +
    +class TestOpenRouterToolSupportHelper:
    +    """Unit tests for _openrouter_model_supports_tools (Kilo port #9068)."""
    +
    +    def test_tools_in_supported_parameters(self):
    +        from hermes_cli.models import _openrouter_model_supports_tools
    +        assert _openrouter_model_supports_tools(
    +            {"id": "x", "supported_parameters": ["temperature", "tools"]}
    +        ) is True
    +
    +    def test_tools_missing_from_supported_parameters(self):
    +        from hermes_cli.models import _openrouter_model_supports_tools
    +        assert _openrouter_model_supports_tools(
    +            {"id": "x", "supported_parameters": ["temperature", "response_format"]}
    +        ) is False
    +
    +    def test_supported_parameters_absent_is_permissive(self):
    +        """Missing field → allow (so older / non-OR gateways still work)."""
    +        from hermes_cli.models import _openrouter_model_supports_tools
    +        assert _openrouter_model_supports_tools({"id": "x"}) is True
    +
    +    def test_supported_parameters_none_is_permissive(self):
    +        from hermes_cli.models import _openrouter_model_supports_tools
    +        assert _openrouter_model_supports_tools({"id": "x", "supported_parameters": None}) is True
    +
    +    def test_supported_parameters_malformed_is_permissive(self):
    +        """Malformed (non-list) value → allow rather than silently drop."""
    +        from hermes_cli.models import _openrouter_model_supports_tools
    +        assert _openrouter_model_supports_tools(
    +            {"id": "x", "supported_parameters": "tools,temperature"}
    +        ) is True
    +
    +    def test_non_dict_item_is_permissive(self):
    +        from hermes_cli.models import _openrouter_model_supports_tools
    +        assert _openrouter_model_supports_tools(None) is True
    +        assert _openrouter_model_supports_tools("anthropic/claude-opus-4.6") is True
    +
    +    def test_empty_supported_parameters_list_drops_model(self):
    +        """Explicit empty list → no tools → drop."""
    +        from hermes_cli.models import _openrouter_model_supports_tools
    +        assert _openrouter_model_supports_tools(
    +            {"id": "x", "supported_parameters": []}
    +        ) is False
    +
     
     class TestFindOpenrouterSlug:
         def test_exact_match(self):