Skip to content

fix(doctor): flag a custom provider whose key_env names an unset variable - #101350

Open
jonpol01 wants to merge 2 commits into
NousResearch:mainfrom
jonpol01:fix/doctor-custom-provider-key-env
Open

jonpol01 wants to merge 2 commits into
NousResearch:mainfrom
jonpol01:fix/doctor-custom-provider-key-env

Conversation

@jonpol01

@jonpol01 jonpol01 commented Sep 2, 2026 •

Copy link
Copy Markdown
Contributor

What does this PR do?

hermes doctor now fails when the active custom provider's key_env names an environment variable that is not set.

Problem

The credential check in hermes_cli/doctor.py (~:1685) is deliberately limited to API-key providers in PROVIDER_REGISTRY, to avoid false positives from get_auth_status(). A named custom provider — a providers: entry or a legacy custom_providers item — is not in the registry, so the check skips it and doctor reports the configuration healthy.

At runtime (hermes_cli/runtime_provider.py:830-870) such a provider's key resolves as key_env → inline api_key → key_cmd. When key_env names a variable that is not set and the entry has no other source, every request fails with an auth error, and the log points at the provider rather than at the missing variable. The tracker carries a long tail of "custom provider key_env → 401" reports (#43586, #44349, #46690, #62254, #67453, #67935, #74561, #83847, #91683, #92124, #100858) — exactly the misconfiguration doctor exists to catch, and it said nothing.

Change

After the existing registry credential check, doctor resolves the active model.provider to its custom entry through the same alias set the runtime resolver uses (custom_provider_aliases: display name, providers.<key>, custom:<key>), and when that entry declares key_env (or the api_key_env alias) but the variable is unset and there is no inline api_key or key_cmd, it fails with the variable's name and where to set it:

✗ model.provider 'mylocal' reads its API key from MYLOCAL_API_KEY, which is not set (set MYLOCAL_API_KEY in ~/.hermes/.env)

Scope, deliberately narrow:

  • active route only — an inactive entry's unset key_env is not this section's business;
  • named custom providers only — the bare provider: custom + model.key_env form is left alone because the runtime does not resolve that key yet (open fix(config): resolve model.key_env in bare custom provider path (#67453) #67554); validating it here would fail a key the runtime never reads;
  • reads the variable through get_env_value (config .env + environment), the same call the existing OpenRouter branch uses;
  • consults the raw providers: entry as well as the compatibility view for api_key / key_cmd: _normalize_custom_provider_entry lists key_cmd as a known key but does not carry it into the view, while the runtime reads it from the raw entry (runtime_provider.py:866) — without this a key_cmd provider would have been flagged falsely (a test pins it).

The registry check's own false-positive guard is untouched: this path never calls get_auth_status.

Related Issue

Related: #67453 (runtime resolution of model.key_env, open PR #67554), #100858, and the "key_env → 401" family listed above. None asks for the doctor check.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • hermes_cli/doctor.py — _custom_provider_entry_for() helper; key_env check after the registry credential block
  • tests/hermes_cli/test_doctor.py — one invariant table through run_doctor: an unset key_env on the active provider is flagged, for both the providers: mapping and the legacy custom_providers list; a set variable, an inline api_key, a key_cmd and an inactive entry stay quiet.
  • website/docs/integrations/providers.md — the Named Custom Providers section says hermes doctor reports such an entry.

How to Test

  1. scripts/run_tests.sh tests/hermes_cli/test_doctor.py -k key_env
  2. Manually: add providers: {mylocal: {base_url: http://127.0.0.1:1234/v1, key_env: MYLOCAL_API_KEY}} and model.provider: mylocal, run hermes doctor with the variable unset → the line above; export it → clean.

Sabotage-checked: with the pre-fix doctor only the two flagged rows fail; with api_key / key_cmd no longer exempting the entry only those two quiet rows fail. Merged onto current main locally: no conflicts, all nine tests/hermes_cli/test_doctor*.py files pass (155 passed). ruff check is clean.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits
  • I searched for existing PRs and issues — key_env, _compatible_custom_providers, "doctor custom provider api key"; fix(config): resolve model.key_env in bare custom provider path (#67453) #67554 fixes the runtime path, nothing adds the doctor check
  • My PR contains only changes related to this fix
  • Ran the doctor suite, ruff check, and the footgun linter; relying on CI for the full matrix
  • I've added tests for my changes — one invariant table (six rows), red on the pre-fix code
  • I've tested on my platform: macOS 27.0 (Darwin 27.0.0), Python 3.11

Documentation & Housekeeping

  • Documentation — website/docs/integrations/providers.md (Named Custom Providers)
  • cli-config.yaml.example — N/A
  • CONTRIBUTING.md / AGENTS.md — N/A
  • Cross-platform — pure config/env inspection; footgun linter clean
  • Tool descriptions/schemas — N/A

@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard area/auth Authentication, OAuth, credential pools area/config Config system, migrations, profiles P2 Medium — degraded but workaround exists sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Sep 2, 2026
@jonpol01
jonpol01 force-pushed the fix/doctor-custom-provider-key-env branch 2 times, most recently from 4bbceae to 1d904cc Compare September 3, 2026 07:59
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference; please use your judgment.

PR 101350 — fix(doctor): flag a custom provider whose key_env names an unset var. Extends doctor's credential check to named custom providers (hermes_cli/doctor.py:1746), resolving the active model.provider through the same alias set as the runtime and consulting both the compat view and the raw providers: mapping (since key_cmd doesn't survive normalization). Only the active provider is flagged; inline api_key/key_cmd suppress the warning. Solid test matrix (unset/set/inline/key_cmd/legacy-list/inactive).

Non-blocking observations:

  • hermes_cli/doctor.py:1749 — the whole block is wrapped in try/except: pass, consistent with surrounding doctor defensiveness; a regression inside the new helper would silently disable the check rather than break doctor. Acceptable for diagnostics, though a debug log would aid future troubleshooting.
  • _custom_provider_entry_for (doctor.py:1255) lowercases for comparison but returns the raw entry; the subsequent raw-mapping lookup also lowercases both sides. Consistent. No concerns.

@jonpol01
jonpol01 force-pushed the fix/doctor-custom-provider-key-env branch from 1d904cc to 9a7bf96 Compare September 6, 2026 15:34
@jonpol01
jonpol01 force-pushed the fix/doctor-custom-provider-key-env branch 2 times, most recently from 5f1eb6e to 20fc1d5 Compare September 18, 2026 18:53
…able

doctor's credential check runs only for providers in PROVIDER_REGISTRY
with auth_type api_key. A named custom provider (a `providers:` entry or a
legacy `custom_providers` item) is not in the registry, so the check
skipped it and doctor reported the configuration healthy. At runtime the
key resolves key_env → inline api_key → key_cmd
(hermes_cli/runtime_provider.py); when key_env names a variable that is
not set and the entry has no other source, every request fails with an
auth error and the user is sent to inspect the provider instead of their
.env — the tracker has a long tail of "custom provider key_env → 401"
reports for exactly this.

After the registry check, resolve the active model.provider to its custom
entry through the same alias set the runtime uses and, when its key_env is
declared but unset and no api_key / key_cmd is present, fail with the
variable's name and where to set it. Only the active route is checked
here; the bare `provider: custom` + model.key_env form is left alone
because the runtime does not resolve that key yet (NousResearch#67554).
@jonpol01
jonpol01 force-pushed the fix/doctor-custom-provider-key-env branch from 20fc1d5 to fec1fed Compare September 20, 2026 05:18
…e doctor warning

Six tests become one table. An unset key_env on the active provider is flagged, for both the
providers: mapping and the legacy custom_providers list; a set variable, an inline api_key, a
key_cmd and an inactive entry stay quiet. The two red rows fail on the pre-fix doctor, and the
api_key/key_cmd rows fail when those fallbacks stop exempting the entry.

Docs: the named custom providers section says hermes doctor reports such an entry.

This branch has not been deployed

No deployments
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:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants