Skip to content

fix(config): resolve model.key_env in bare custom provider path (#67453) - #67554

Closed
JonthanaHanh wants to merge 1 commit into
NousResearch:mainfrom
JonthanaHanh:fix/custom-provider-key-env-resolution
Closed

JonthanaHanh wants to merge 1 commit into
NousResearch:mainfrom
JonthanaHanh:fix/custom-provider-key-env-resolution

Conversation

@JonthanaHanh

Copy link
Copy Markdown
Contributor

Summary

When provider: custom is configured with model.key_env (instead of an inline api_key), the API key is correctly resolved for the first session after boot but fails with 403 for every subsequent session.

Root Cause

The bare provider: custom + model.base_url path in resolve_runtime_provider() (lines 940-972) builds an api_key_candidates list but never reads key_env from the model config. It only checks:

  1. explicit_api_key (always None when called from _resolve_runtime_agent_kwargs)
  2. OPENAI_API_KEY (gated on OpenAI host)
  3. OPENROUTER_API_KEY (gated on OpenRouter host)
  4. _host_derived_api_key (derived from hostname)

The model.key_env field (e.g. SCW_LLM_API_KEY) is completely ignored. The first session may succeed through a different resolution path (e.g. _host_derived_api_key matching), but subsequent sessions fail because the key is never re-resolved from the configured env var.

Fix

Read model.key_env from config and resolve the env var via _getenv(), adding it to the api_key_candidates list after explicit_api_key and before host-gated fallbacks. This mirrors the named-custom-provider path at line 1013 which already reads key_env.

Testing

  • All 151 tests/hermes_cli/test_runtime_provider*.py tests pass
  • py_compile clean

Fixes #67453

…Research#67453)

The bare  +  path in
resolve_runtime_provider() did not read key_env from the model config.
It only checked explicit_api_key, OPENAI_API_KEY, OPENROUTER_API_KEY,
and host-derived env vars.

When a user configures:
  model:
    provider: custom
    key_env: SCW_LLM_API_KEY
    base_url: https://api.scaleway.ai/.../v1

The key was resolved via _host_derived_api_key on the first call (which
may have found it through a different path), but subsequent sessions
failed with 403 because the key_env was never consulted directly.

Add model.key_env resolution to the api_key_candidates list so the key
is re-resolved from the env var on every request.

Fixes NousResearch#67453
@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/auth Authentication, OAuth, credential pools area/config Config system, migrations, profiles sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades needs-decision Awaiting maintainer decision before any implementation labels Jul 19, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tracing the missing key_env lookup. The current change does not reach the API-server path reported in #67453.

Problems

  • The edited branch requires explicit_base_url (hermes_cli/runtime_provider.py:940). API-server creation resolves credentials through _resolve_runtime_agent_kwargs() (gateway/platforms/api_server.py:1746), which calls resolve_runtime_provider() with no explicit arguments (gateway/run.py:2008). That path reaches _resolve_openrouter_runtime(), whose candidates at hermes_cli/runtime_provider.py:1139-1149 still omit model.key_env.
  • The PR changes no tests. A regression should cover plain model.provider: custom + model.base_url + model.key_env; the linked issue's two-request API-server sequence also needs a targeted repro before claiming the repeat-request failure is fixed.

Suggested changes

  • Apply the lookup in the configured-custom resolution path (or factor the candidate construction so both paths use it), then add the corresponding regression coverage.

Automated hermes-sweeper review.

# on every request — not just the first session after boot (#67453).
_model_cfg_key_env = str(
(_get_model_config().get("key_env") or "").strip()
)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This lookup is only in the explicit_base_url direct-alias branch. The API-server flow in #67453 resolves with no explicit arguments, then reaches _resolve_openrouter_runtime() where model.key_env is still absent from the candidate chain; please move/share this logic and add a regression test for the plain configured-custom path.

@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Summary

Seventeen PRs reference this issue complex: four address model-alias dispatch, twelve address distinct custom-provider credential-resolution paths, and #67554 adds model.key_env only to the explicit-base-URL branch rather than the API-server path reported by #67453. The diffs therefore share configuration-routing themes but do not represent one interchangeable fix.

Related pull requests

Duplicates

Alias cluster: closed #29198 and stale #30716 overlap with broader #19213, while #62534 is the focused current CLI implementation. Credential cluster: closed #44669, #44684, and #46023 duplicate #44667's runtime alias change; open #12443, #44868, and #62507 overlap there, while #43675 and #43704 are complementary halves substantially combined by #45873.

Suggested consolidation

Keep #67554 open with a salvage path, consistent with its contributor review: move or share the profile-scoped model.key_env/api_key_env lookup into the configured-custom _resolve_openrouter_runtime path and add both a public resolver regression and the reported two-request API-server test. Keep recorded best-fix PRs #19213, #45873, #62507, and #62534 open only for their documented current-head salvage work; #30716 can be closed as a duplicate of #62534 because its stale, testless constructor diff is subsumed, while the already-closed credential duplicates need no further state action.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I67453(["issue #67453 (open)"])
    P67554["PR #67554 (open)"]
    P67554 -.->|partial| I67453
    class I67453 open
    class P67554 open
    class P67554 target
    click I67453 "https://github.com/NousResearch/hermes-agent/issues/67453"
    click P67554 "https://github.com/NousResearch/hermes-agent/pull/67554"
Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 17 pull requests and 8 issues in this complex. Each diff was read against this issue; Assessment working set: 78 kB of PR diffs, 60 kB of issue/PR text, 49 kB of discussion (61 comments), 60 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

jonpol01 added a commit to jonpol01/hermes-agent that referenced this pull request Sep 3, 2026
…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 added a commit to jonpol01/hermes-agent that referenced this pull request Sep 6, 2026
…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 added a commit to jonpol01/hermes-agent that referenced this pull request Sep 18, 2026
…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 added a commit to jonpol01/hermes-agent that referenced this pull request Sep 18, 2026
…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).
teknium1 added a commit that referenced this pull request Sep 19, 2026
…key-resolution

fix(providers): bare custom provider sends the key named by model.key_env; unset key_env is warned about (#67453, salvage #67554)
pull Bot pushed a commit to dot-agi/hermes-agent that referenced this pull request Sep 19, 2026
…et key_env is logged instead of laundered into no-key-required

`model.provider: custom` + `model.base_url` + `model.key_env` (the wizard's bare-custom
shape) resolved through `_resolve_openrouter_runtime`, whose candidate list only knew
`model.api_key` and the host-gated OPENAI/OPENROUTER env keys — `model.key_env` was never
read, so every request went out as `Bearer no-key-required` and the endpoint returned
401/403 in tens of milliseconds (NousResearch#67453). The same rung is what the API server platform
resolves on each request, which is why the reporter saw it on every request.

- `_model_cfg_key_env_for()` supplies the declared variable's value on the bare-custom and
  direct-alias rungs, only when the target base_url IS the configured `model.base_url`
  (a CUSTOM_BASE_URL or alias endpoint elsewhere never receives that key).
- `_key_env_secret()` is the one key_env/api_key_env reader for custom blocks (model block
  and custom_providers entries): a declared variable that resolves to nothing is now
  WARNING-logged before the `no-key-required` substitution, so a misnamed/unexported var
  points at the Hermes config instead of the provider's IAM. Blocks with no key_env stay
  silent — that is the keyless local-server configuration.
- `is_output_cap_error()` recognises "max_completion_tokens is limited to N" (Scaleway), so
  the budget step-down triggers instead of the compressor (second atom of NousResearch#67453).

Supersedes NousResearch#67554 (@JonthanaHanh), which added the same lookup on the explicit-base_url
branch only.
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks @JonthanaHanh — the work in this PR has landed on main via:

Your contribution is credited there (cherry-picked authorship / co-author trailer or credit in the PR body; see the linked PR for what was kept and what was trimmed). Closing this one as landed / superseded so the backlog reflects reality. If something in your original diff is still missing on current main, please comment and we'll reopen or follow up.

@teknium1 teknium1 closed this Sep 19, 2026
jonpol01 added a commit to jonpol01/hermes-agent that referenced this pull request Sep 20, 2026
…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).
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 needs-decision Awaiting maintainer decision before any implementation P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug] custom provider key_env only resolved for the first session after boot — all subsequent sessions get 401/403

4 participants