Skip to content

fix(providers): support api_key_env alongside key_env for custom providers - #12443

Open
Fatty911 wants to merge 2 commits into
NousResearch:mainfrom
Fatty911:fix-legacy-api-key-env-conflict
Open

Fatty911 wants to merge 2 commits into
NousResearch:mainfrom
Fatty911:fix-legacy-api-key-env-conflict

Conversation

@Fatty911

Copy link
Copy Markdown
Contributor

What does this PR do?

Resolves a legacy field naming conflict for custom providers. The documentation and config examples frequently mention using api_key_env to reference environment variables for authentication with custom OpenAI-compatible endpoints (and the auxiliary block natively uses it). However, hermes_cli/config.py and hermes_cli/runtime_provider.py strictly looked for key_env, resulting in silent fallback to a blank token. This caused unexplained 401 HTTP errors when resolving custom endpoints (like SiliconFlow, DMXAPI, or Bedrock gateways).

This change adds api_key_env as a fallback when retrieving the provider's token, making custom provider authentication robust and matching user expectations.

Related Issue

Fixes silent token omissions resulting in 401 Unauthorized API responses on custom endpoints.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • Updated _normalize_custom_provider_entry in hermes_cli/config.py to also read api_key_env.
  • Updated _get_named_custom_provider and _resolve_named_custom_runtime in hermes_cli/runtime_provider.py to accept api_key_env when resolving environment keys.

How to Test

  1. Add a custom provider in config.yaml using api_key_env: SOME_API_KEY (instead of key_env).
  2. Point your .env SOME_API_KEY to a valid API token for a custom gateway.
  3. Call an LLM with this provider. The request successfully injects the environment variable into the LLM authentication header without a 401 error.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: Linux

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

@Fatty911
Fatty911 force-pushed the fix-legacy-api-key-env-conflict branch from 0cba04c to 5d0775d Compare April 19, 2026 07:09
@Artem151193

Copy link
Copy Markdown

Review summary: FIXED 0 / DEFERRED 0 / NOISE 0 / ACTIONABLE 0.

CI blocker: required check build-and-push failed, and test was cancelled after the failure. The failing job hits a permissions error in the container test step (mkdir: cannot create directory '/opt/data/...': Permission denied). This PR is not merge-ready until the workflow issue is resolved or explicitly waived.

@Artem151193

Copy link
Copy Markdown

Automated review check: BLOCKED.

The test job failed, so this PR is not merge-ready yet. build-and-push, check-attribution, and e2e passed; please fix the failing test job and rerun CI.

@Fatty911
Fatty911 force-pushed the fix-legacy-api-key-env-conflict branch 19 times, most recently from a6f1c6f to c0c8fa2 Compare April 20, 2026 09:43
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard area/config Config system, migrations, profiles labels Apr 23, 2026
@Fatty911
Fatty911 force-pushed the fix-legacy-api-key-env-conflict branch from c0c8fa2 to 687a485 Compare May 4, 2026 17:15
…iders

Accept api_key_env as a fallback when reading the key_env field in
_get_named_custom_provider and _resolve_named_custom_runtime, matching
the documented config format and the auxiliary block convention.
@Fatty911
Fatty911 force-pushed the fix-legacy-api-key-env-conflict branch from 687a485 to c2971d2 Compare May 4, 2026 17:23
@Fatty911

Fatty911 commented May 5, 2026

Copy link
Copy Markdown
Contributor Author

what about now?

…env-conflict

# Conflicts:
#	hermes_cli/runtime_provider.py
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for tracking the api_key_env compatibility gap. The core runtime issue still exists on current main: hermes_cli/runtime_provider.py:662 reads only key_env while scanning new-style providers: entries, and the resulting named-provider runtime consults that value at hermes_cli/runtime_provider.py:1007.

Problems

  • The same providers: schema remains incomplete in hermes_cli/providers.py:597-608: resolve_user_provider() reads only key_env, and resolve_provider_full() uses it at hermes_cli/providers.py:728-746. An api_key_env-only entry therefore still exposes no credential variable through that path.
  • The PR diff changes only hermes_cli/runtime_provider.py and adds no regression test. Main's existing alias tests at tests/hermes_cli/test_runtime_provider_resolution.py:2530-2569 cover normalization, not direct runtime resolution of a raw providers: entry.

Suggested changes

  • Preserve current main's profile-scoped _getenv() while salvaging the runtime hunk.
  • Extend the alias fallback to hermes_cli/providers.py:597 and add a providers:-entry regression test for runtime resolution, provider-definition resolution, and key_env precedence.

Automated hermes-sweeper review.

@alt-glitch alt-glitch added the area/auth Authentication, OAuth, credential pools label Jul 12, 2026
@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 12, 2026
@alt-glitch alt-glitch removed the sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades label Jul 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools area/config Config system, migrations, profiles comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants