fix(auxiliary_client): demote the 2 sibling routing fall-throughs too (review)
Phase 2c review flagged that only 2 of the 4 structurally-identical
resolve_provider_client routing dead-ends were demoted. Complete the bug-class:
also demote+dedup the external-process ('not directly supported') and OAuth
('not directly supported, try auto') fall-throughs, keyed by provider name, so
none of the four dead-ends spam WARNING on a retry loop.
Add direct tests for the unhandled-auth_type and OAuth dedup paths via a
monkeypatched PROVIDER_REGISTRY (the review noted these were unverified).
Mutation-checked: reverting either sibling demotion fails its test.
This commit is contained in:
parent
c0d3ceb17e
commit
9cf47fef54
2 changed files with 79 additions and 4 deletions
|
|
@ -121,6 +121,11 @@ logger = logging.getLogger(__name__)
|
|||
# them independently.
|
||||
_LOGGED_UNKNOWN_PROVIDER_KEYS: set = set()
|
||||
_LOGGED_UNHANDLED_AUTHTYPE_KEYS: set = set()
|
||||
# Same treatment for the two "registered provider, unsupported sub-branch"
|
||||
# routing dead-ends — external-process and OAuth providers that fall through
|
||||
# with no matching handler. Keyed by provider name.
|
||||
_LOGGED_UNSUPPORTED_EXTPROC_KEYS: set = set()
|
||||
_LOGGED_UNSUPPORTED_OAUTH_KEYS: set = set()
|
||||
|
||||
|
||||
def _openai_http_client_kwargs(
|
||||
|
|
@ -4495,8 +4500,10 @@ def resolve_provider_client(
|
|||
logger.debug("resolve_provider_client: %s (%s)", provider, final_model)
|
||||
return (_to_async_client(client, final_model, is_vision=is_vision) if async_mode
|
||||
else (client, final_model))
|
||||
logger.warning("resolve_provider_client: external-process provider %s not "
|
||||
"directly supported", provider)
|
||||
if provider not in _LOGGED_UNSUPPORTED_EXTPROC_KEYS:
|
||||
_LOGGED_UNSUPPORTED_EXTPROC_KEYS.add(provider)
|
||||
logger.debug("resolve_provider_client: external-process provider %s not "
|
||||
"directly supported", provider)
|
||||
return None, None
|
||||
|
||||
elif pconfig.auth_type == "aws_sdk":
|
||||
|
|
@ -4541,8 +4548,10 @@ def resolve_provider_client(
|
|||
if provider == "xai-oauth":
|
||||
return resolve_provider_client("xai-oauth", model, async_mode)
|
||||
# Other OAuth providers not directly supported
|
||||
logger.warning("resolve_provider_client: OAuth provider %s not "
|
||||
"directly supported, try 'auto'", provider)
|
||||
if provider not in _LOGGED_UNSUPPORTED_OAUTH_KEYS:
|
||||
_LOGGED_UNSUPPORTED_OAUTH_KEYS.add(provider)
|
||||
logger.debug("resolve_provider_client: OAuth provider %s not "
|
||||
"directly supported, try 'auto'", provider)
|
||||
return None, None
|
||||
|
||||
# Demoted from logger.warning to debug; dedup keyed on (auth_type,
|
||||
|
|
|
|||
|
|
@ -50,3 +50,69 @@ class TestUnknownProviderDedup:
|
|||
if "unknown provider" in r.getMessage()
|
||||
]
|
||||
assert len(recs) == 2
|
||||
|
||||
|
||||
class TestUnhandledAuthTypeDedup:
|
||||
def setup_method(self):
|
||||
ac._LOGGED_UNHANDLED_AUTHTYPE_KEYS.clear()
|
||||
|
||||
def test_unhandled_auth_type_logs_debug_once_not_warning(self, caplog, monkeypatch):
|
||||
import hermes_cli.auth as auth
|
||||
from hermes_cli.auth import ProviderConfig
|
||||
|
||||
# A registered provider whose auth_type matches no handled branch →
|
||||
# the terminal "unhandled auth_type" fall-through.
|
||||
bogus = ProviderConfig(
|
||||
id="bogus_authtype",
|
||||
name="Bogus",
|
||||
auth_type="totally_unhandled_scheme",
|
||||
)
|
||||
patched = dict(auth.PROVIDER_REGISTRY)
|
||||
patched["bogus_authtype"] = bogus
|
||||
monkeypatch.setattr(auth, "PROVIDER_REGISTRY", patched)
|
||||
|
||||
with caplog.at_level(logging.DEBUG, logger="agent.auxiliary_client"):
|
||||
client, model = resolve_provider_client("bogus_authtype", "")
|
||||
resolve_provider_client("bogus_authtype", "") # repeat → suppressed
|
||||
|
||||
assert (client, model) == (None, None)
|
||||
recs = [
|
||||
r for r in caplog.records
|
||||
if "unhandled auth_type" in r.getMessage()
|
||||
]
|
||||
# Two calls, one DEBUG record, never WARNING.
|
||||
assert len(recs) == 1
|
||||
assert recs[0].levelno == logging.DEBUG
|
||||
assert not any(r.levelno >= logging.WARNING for r in recs)
|
||||
|
||||
|
||||
class TestUnsupportedOAuthDedup:
|
||||
def setup_method(self):
|
||||
ac._LOGGED_UNSUPPORTED_OAUTH_KEYS.clear()
|
||||
|
||||
def test_unsupported_oauth_provider_logs_debug_once(self, caplog, monkeypatch):
|
||||
import hermes_cli.auth as auth
|
||||
from hermes_cli.auth import ProviderConfig
|
||||
|
||||
# A registered oauth_* provider that is not one of the directly-handled
|
||||
# names (nous / openai-codex / xai-oauth) → the OAuth dead-end branch.
|
||||
bogus = ProviderConfig(
|
||||
id="bogus_oauth",
|
||||
name="BogusOAuth",
|
||||
auth_type="oauth_device_code",
|
||||
)
|
||||
patched = dict(auth.PROVIDER_REGISTRY)
|
||||
patched["bogus_oauth"] = bogus
|
||||
monkeypatch.setattr(auth, "PROVIDER_REGISTRY", patched)
|
||||
|
||||
with caplog.at_level(logging.DEBUG, logger="agent.auxiliary_client"):
|
||||
resolve_provider_client("bogus_oauth", "")
|
||||
resolve_provider_client("bogus_oauth", "")
|
||||
|
||||
recs = [
|
||||
r for r in caplog.records
|
||||
if "OAuth provider" in r.getMessage() and "not " in r.getMessage()
|
||||
]
|
||||
assert len(recs) == 1
|
||||
assert recs[0].levelno == logging.DEBUG
|
||||
assert not any(r.levelno >= logging.WARNING for r in recs)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue