From 9efe5b07eb3fa7903de4d98721d4a7aea5b1e121 Mon Sep 17 00:00:00 2001 From: srojk34 <286497132+srojk34@users.noreply.github.com> Date: Wed, 1 Jul 2026 14:52:37 +0300 Subject: [PATCH] fix(auxiliary_client): dedup 3 remaining resolve_provider_client dead-ends Same anti-pattern the previous two dedup passes fixed (unknown provider, unhandled auth_type, unsupported external-process/OAuth provider): three more static-misconfiguration fall-throughs in the named-custom-provider and copilot-acp branches logged via logger.warning on every call for a persistently-misconfigured provider, spamming the logs forever until the user edits config.yaml: - named custom provider with no resolvable api_key - named custom provider with no base_url - copilot-acp requested with no model configured anywhere Demote all three to logger.debug with the same per-process dedup-set pattern already established in this file (first occurrence still surfaces, identical repeats suppressed for the process lifetime). --- agent/auxiliary_client.py | 40 ++++++--- .../test_auxiliary_client_resolve_dedup.py | 83 +++++++++++++++++++ 2 files changed, 110 insertions(+), 13 deletions(-) diff --git a/agent/auxiliary_client.py b/agent/auxiliary_client.py index 3ce0308cea7f2..503511870a24c 100644 --- a/agent/auxiliary_client.py +++ b/agent/auxiliary_client.py @@ -126,6 +126,13 @@ def __repr__(self): # with no matching handler. Keyed by provider name. _LOGGED_UNSUPPORTED_EXTPROC_KEYS: set = set() _LOGGED_UNSUPPORTED_OAUTH_KEYS: set = set() +# Same treatment for the remaining static-misconfiguration dead-ends in the +# named-custom-provider and copilot-acp branches: a provider/entry with a +# permanently-missing api_key, base_url, or model repeats the identical +# warning on every call until the user edits config.yaml. +_LOGGED_NAMED_CUSTOM_NOKEY_KEYS: set = set() +_LOGGED_NAMED_CUSTOM_NOBASEURL_KEYS: set = set() +_LOGGED_COPILOT_ACP_NOMODEL_KEYS: set = set() def _openai_http_client_kwargs( @@ -4220,12 +4227,15 @@ def _wrap_if_needed(client_obj, final_model_str: str, base_url_str: str = "", custom_key = os.getenv(custom_key_env, "").strip() custom_key = custom_key or "no-key-required" if custom_key == "no-key-required": - logger.warning( - "resolve_provider_client: named custom provider %r has no resolvable " - "api_key — request will be sent with placeholder no-key-required " - "and will 401 on auth-required endpoints", - custom_entry.get("name") or provider, - ) + _custom_nokey_key = custom_entry.get("name") or provider + if _custom_nokey_key not in _LOGGED_NAMED_CUSTOM_NOKEY_KEYS: + _LOGGED_NAMED_CUSTOM_NOKEY_KEYS.add(_custom_nokey_key) + logger.debug( + "resolve_provider_client: named custom provider %r has no resolvable " + "api_key — request will be sent with placeholder no-key-required " + "and will 401 on auth-required endpoints", + _custom_nokey_key, + ) # An explicit per-task api_mode override (from _resolve_task_provider_model) # wins; otherwise fall back to what the provider entry declared. entry_api_mode = (api_mode or custom_entry.get("api_mode") or "").strip() @@ -4300,9 +4310,11 @@ def _wrap_if_needed(client_obj, final_model_str: str, base_url_str: str = "", client = _wrap_if_needed(client, final_model, raw_base_for_wrap, custom_key) return (_to_async_client(client, final_model, is_vision=is_vision) if async_mode else (client, final_model)) - logger.warning( - "resolve_provider_client: named custom provider %r has no base_url", - provider) + if provider not in _LOGGED_NAMED_CUSTOM_NOBASEURL_KEYS: + _LOGGED_NAMED_CUSTOM_NOBASEURL_KEYS.add(provider) + logger.debug( + "resolve_provider_client: named custom provider %r has no base_url", + provider) return None, None except ImportError: pass @@ -4478,10 +4490,12 @@ def _wrap_if_needed(client_obj, final_model_str: str, base_url_str: str = "", command = str(creds.get("command", "")).strip() or None args = list(creds.get("args") or []) if not final_model: - logger.warning( - "resolve_provider_client: copilot-acp requested but no model " - "was provided or configured" - ) + if provider not in _LOGGED_COPILOT_ACP_NOMODEL_KEYS: + _LOGGED_COPILOT_ACP_NOMODEL_KEYS.add(provider) + logger.debug( + "resolve_provider_client: copilot-acp requested but no model " + "was provided or configured" + ) return None, None if not api_key or not base_url: logger.warning( diff --git a/tests/agent/test_auxiliary_client_resolve_dedup.py b/tests/agent/test_auxiliary_client_resolve_dedup.py index 1bb7bbd94b95a..5ef24ee30da84 100644 --- a/tests/agent/test_auxiliary_client_resolve_dedup.py +++ b/tests/agent/test_auxiliary_client_resolve_dedup.py @@ -116,3 +116,86 @@ def test_unsupported_oauth_provider_logs_debug_once(self, caplog, monkeypatch): assert len(recs) == 1 assert recs[0].levelno == logging.DEBUG assert not any(r.levelno >= logging.WARNING for r in recs) + + +class TestNamedCustomProviderNoKeyDedup: + """A named custom provider (config.yaml providers/custom_providers entry) + with no resolvable api_key repeats the identical warning on every call + until the user edits config — same dead-end shape as the OAuth/extproc + branches above.""" + + def setup_method(self): + ac._LOGGED_NAMED_CUSTOM_NOKEY_KEYS.clear() + + def test_no_key_logs_debug_once_not_warning(self, caplog, monkeypatch): + import hermes_cli.runtime_provider as rp + + entry = {"name": "my-custom", "base_url": "https://custom.example/v1"} + monkeypatch.setattr(rp, "_get_named_custom_provider", lambda _p: entry) + + with caplog.at_level(logging.DEBUG, logger="agent.auxiliary_client"): + resolve_provider_client("my-custom", "some-model") + resolve_provider_client("my-custom", "some-model") # repeat → suppressed + + recs = [ + r for r in caplog.records + if "has no resolvable" in r.getMessage() + ] + assert len(recs) == 1 + assert recs[0].levelno == logging.DEBUG + assert not any(r.levelno >= logging.WARNING for r in recs) + + +class TestNamedCustomProviderNoBaseUrlDedup: + def setup_method(self): + ac._LOGGED_NAMED_CUSTOM_NOBASEURL_KEYS.clear() + + def test_no_base_url_logs_debug_once_not_warning(self, caplog, monkeypatch): + import hermes_cli.runtime_provider as rp + + entry = {"name": "my-custom-2", "api_key": "sk-test"} + monkeypatch.setattr(rp, "_get_named_custom_provider", lambda _p: entry) + + with caplog.at_level(logging.DEBUG, logger="agent.auxiliary_client"): + client, model = resolve_provider_client("my-custom-2", "some-model") + resolve_provider_client("my-custom-2", "some-model") # repeat → suppressed + + assert (client, model) == (None, None) + recs = [ + r for r in caplog.records + if "has no base_url" in r.getMessage() + ] + assert len(recs) == 1 + assert recs[0].levelno == logging.DEBUG + assert not any(r.levelno >= logging.WARNING for r in recs) + + +class TestCopilotAcpNoModelDedup: + def setup_method(self): + ac._LOGGED_COPILOT_ACP_NOMODEL_KEYS.clear() + + def test_no_model_logs_debug_once_not_warning(self, caplog, monkeypatch): + import hermes_cli.auth as auth + + monkeypatch.setattr(ac, "_read_main_model", lambda: "") + monkeypatch.setattr(ac, "_get_aux_model_for_provider", lambda _p: "") + # The CLI binary isn't installed on the test machine; resolving + # external-process credentials would otherwise raise before this + # branch's own model check runs. + monkeypatch.setattr( + auth, "resolve_external_process_provider_credentials", + lambda _p: {"api_key": "k", "base_url": "http://local", "command": "copilot", "args": []}, + ) + + with caplog.at_level(logging.DEBUG, logger="agent.auxiliary_client"): + client, model = resolve_provider_client("copilot-acp", "") + resolve_provider_client("copilot-acp", "") # repeat → suppressed + + assert (client, model) == (None, None) + recs = [ + r for r in caplog.records + if "no model was provided or configured" in r.getMessage() + ] + assert len(recs) == 1 + assert recs[0].levelno == logging.DEBUG + assert not any(r.levelno >= logging.WARNING for r in recs)