fix(providers): accept camelCase aliases and validate URL scheme in providers: config (#9332)#9373
Closed
draix wants to merge 1 commit into
Closed
Conversation
…roviders: config (NousResearch#9332) Two bugs in resolve_user_provider() caused silent misconfigurations: 1. Only snake_case keys recognised Hand-written YAML often uses camelCase — especially by developers coming from JavaScript/TypeScript environments. 'baseUrl', 'keyEnv' were silently dropped, leaving base_url empty and api_key_env_vars empty, producing 'Bearer no-key-required' errors with no hint about the actual cause. Fix: add camelCase aliases resolved after the primary snake_case keys. Resolution order (first non-empty wins): base_url > baseUrl > url > api (URL field) key_env > keyEnv > api_key_env (env-var field) snake_case always takes precedence when both forms are present. 2. Non-URL strings silently accepted as endpoint URLs The 'api' field lookup accepts the first non-empty string regardless of whether it looks like a URL. A bare string like 'openai-reverse-proxy' was silently accepted as the endpoint, causing cryptic downstream connection errors. Fix: after resolving the URL candidate, check for http:// or https:// prefix. If the string doesn't have a recognisable scheme, log a WARNING with the offending value and the accepted key names, and clear the field so the provider falls back to its built-in defaults. Testing ------- 20 new tests in tests/hermes_cli/test_providers_config_parsing.py: - TestSnakeCaseKeys (4): existing behavior preserved - TestCamelCaseAliases (5): baseUrl, keyEnv, both together, precedence - TestURLValidation (5): non-URL rejection + warning, valid URLs accepted - TestEdgeCases (6): None entry, empty dict, missing key, name, transport Fixes NousResearch#9332
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two bugs in
resolve_user_provider()(hermes_cli/providers.py) caused silent misconfigurations in theproviders:keyed config section.Bug 1: camelCase keys silently dropped
Hand-written YAML often uses camelCase — especially by developers coming from JavaScript/TypeScript environments.
baseUrl,apiKey,keyEnvwere silently dropped, leavingbase_urlempty andapi_key_env_varsempty.User-visible symptom:
Bearer no-key-requiredin outgoing requests and crypticAPIConnectionErrorwith no hint that the config key is wrong.Fix: Add camelCase aliases with snake_case taking precedence:
Resolution order (first non-empty wins):
base_url>baseUrl>url>apiBug 2: Non-URL strings silently accepted as endpoints
The
apifield lookup accepted the first non-empty string regardless of whether it looks like a URL. A bare string likeopenai-reverse-proxywas silently accepted as the endpoint, causing confusing downstream connection failures.Fix: After resolving the URL candidate, check for
http://orhttps://prefix. If the string doesn't have a recognisable scheme, emit aWARNINGwith the offending value and the accepted key names, and clear the field.Testing
20 new tests in
tests/hermes_cli/test_providers_config_parsing.py:TestSnakeCaseKeysTestCamelCaseAliasesbaseUrl,keyEnv, both together, precedence orderTestURLValidationTestEdgeCasesAll 20 pass.
Fixes #9332