fix(aux): use auth_type-aware error message for missing credentials (#56810) - #56843
Tranquil-Flow wants to merge 2 commits into
Conversation
…ousResearch#56810) The auxiliary client path raised a misleading 'Provider X is set in config.yaml but no API key was found. Set the X_API_KEY environment variable' message for any explicit provider whose client failed to build. That is wrong for providers that authenticate via OAuth, ADC, or SDK chains (vertex, bedrock, openai-codex, external_process, ...) because it points the user at a nonexistent env var and blocks diagnosis. Add _aux_missing_credentials_error() which looks up the provider's auth_type in PROVIDER_REGISTRY and returns an actionable message for each known auth type: api_key (existing), vertex (ADC / gcloud auth application-default login), aws_sdk (aws configure / IAM), oauth_* ('hermes model' to authorize), external_process (binary on PATH), and a safe generic message for unknown / unregistered types that does NOT claim an API key env var exists. Wire the helper at both call_llm (sync) and async_call_llm (async) raise sites. The configured-fallback path is preserved as-is, so users with a fallback chain still get the chain used (existing behaviour, regression-tested).
6d2ba6e to
2ea0931
Compare
|
Author-email amend (force-push |
Duplicate of #56842 (earlier open PR) — both fix #56810 by adding an |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for covering both synchronous and asynchronous auxiliary error paths; current main still has the misleading hardcoded message at agent/auxiliary_client.py:6725 and :7345.
Problems
- The new lookup at
agent/auxiliary_client.py:5859only readsPROVIDER_REGISTRY. Current main's extension loop admits onlyapi_keyprofiles (hermes_cli/auth.py:447-477), while the bundled Vertex profile isauth_type="vertex"(plugins/model-providers/vertex/__init__.py:65-73). Consequently, Vertex falls through to the helper's generic message instead of the proposed ADC guidance. - The new tests inject a fake registry containing Vertex, so they do not cover this production registry gap.
Suggested changes
- Fall back to the canonical provider-profile lookup when
PROVIDER_REGISTRYhas no entry, or include the required Vertex registry registration; then add a regression using the real registry path.
Automated hermes-sweeper review.
| auth_type: Optional[str] = None | ||
| try: | ||
| from hermes_cli.auth import PROVIDER_REGISTRY | ||
| pconfig = PROVIDER_REGISTRY.get(pid) |
There was a problem hiding this comment.
PROVIDER_REGISTRY does not contain Vertex on current main: its auto-extension skips all non-api_key profiles (hermes_cli/auth.py:455), even though the bundled Vertex profile declares auth_type="vertex". This makes the reported Vertex case take the generic branch rather than the ADC-specific branch. Please fall back to the canonical provider-profile registry here, or add the corresponding Vertex registry admission and cover it without a fake registry.
What
Fixes a misleading
RuntimeErrorraised by the auxiliary client when an explicit provider's client build fails. The existing message hardcoded a<X>_API_KEYenvironment variable that does not exist for non-API-key providers (vertex ADC, bedrock AWS SDK, oauth_device_code, external_process), pointing users at a nonexistent remediation and blocking diagnosis.Why
Issue #56810 reports: configuring
auxiliary.compression.provider: vertexwith no GCP credentials raises "Set the VERTEX_API_KEY environment variable" — butVERTEX_API_KEYdoes not exist. The user needs to rungcloud auth application-default login(or setGOOGLE_APPLICATION_CREDENTIALS, or configure vertex credentials in config.yaml).Same bug class affects bedrock (AWS credentials, not API key), oauth-* providers (
hermes modelto authorize), and external-process backends (binary on PATH).How
New helper
_aux_missing_credentials_error(provider_id)inagent/auxiliary_client.py:auth_typeinPROVIDER_REGISTRY(defensive try/except — never raises)RuntimeErrorwith an actionable message for the actual auth mechanism:api_key→ existing "Set the X_API_KEY environment variable" message (preserved)vertex→ "gcloud auth application-default login / GOOGLE_APPLICATION_CREDENTIALS / vertex config"aws_sdk→ "aws configure / AWS_ACCESS_KEY_ID + AWS_SECRET_ACCESS_KEY / IAM roles / SSO"oauth_device_code/oauth_external→ "hermes modelto authorize"external_process→ "verify the binary is installed and on PATH"Both raise sites (sync
call_llmat line 6022 and asyncasync_call_llmat line 6592) replaced withraise _aux_missing_credentials_error(_explicit).The configured-fallback path (
_try_configured_fallback_for_unavailable_client) is preserved as-is — when a fallback chain is configured, the fallback is still tried first, only the error after fallback-exhaustion gets the new message.Tests
tests/agent/test_auxiliary_auth_type_hint_56810.py— 8 regression tests, all production-path (mock_get_cached_clientto return(None, None), drive realcall_llm/async_call_llm):TestVertexAuxClientErrorMessage— vertex + bedrock × sync + async (4 tests, parameterized)TestApiKeyProviderStillHelpful— openai + anthropic still get the existing "Set X_API_KEY" message (2 tests)TestUnknownAuthTypeStillActionable— unknown auth_type does NOT lie about API keys (1 test)TestFallbackChainStillWorks— fallback chain still takes precedence over the error (1 test)Verified
git checkout 88d1d6206f -- agent/auxiliary_client.pythen run new tests → 5 of 8 fail with BEDROCK_API_KEY / VERTEX_API_KEY / CUSTOM-FUTURE-PROVIDER_API_KEY in error messages (3 regression guards correctly pass on the buggy code)test_auxiliary_client.py+test_compress_focus.py+test_compression_concurrent_fork.py. One unrelated flake (TestCodexAuxiliaryAdapterTimeout::test_enforces_total_timeout_while_stream_keeps_emitting_events) — pre-existing on upstream/main, confirmed by retry.Files
agent/auxiliary_client.py— +75/-10 (1 helper + 2 raise-site swaps)tests/agent/test_auxiliary_auth_type_hint_56810.py— new file, +266 lines (8 regression tests)Out of scope
resolve_vision_provider_clienthas its own (correct) message)PROVIDER_REGISTRYitself (this fix is a consumer of the registry's auth_type field; the data must be populated by provider-registration PRs like fix(vertex,moa): register vertex in PROVIDER_REGISTRY and HERMES_OVERLAYS #56688)Competitor analysis
No open PR addresses the same misleading-error symptom. Closest work in this area:
PROVIDER_REGISTRY(complementary — makes the lookup succeed)Auto-published by Moonsong via Path B automated pipeline.