Skip to content

fix(config): preserve api_key during provider migration for unauthenticated servers - #29576

Open
chimpera wants to merge 1 commit into
NousResearch:mainfrom
chimpera:fix/config-migration-preserve-api-key
Open

fix(config): preserve api_key during provider migration for unauthenticated servers#29576
chimpera wants to merge 1 commit into
NousResearch:mainfrom
chimpera:fix/config-migration-preserve-api-key

Conversation

@chimpera

Copy link
Copy Markdown
Contributor

Summary

  • The v11→v12 config migration (custom_providers list → providers dict) stripped api_key values matching "no-key" or "no-key-required" from migrated entries
  • These are legitimate values used by self-hosted backends (llama.cpp, Ollama, vLLM, custom proxies) that expose /v1/models but don't require authentication
  • Without the api_key, model discovery is skipped entirely (see model_switch.py Section 3), causing the /model picker to only show default_model instead of the full catalog
  • Users who add a placeholder api_key to work around this lose it on every config migration triggered by an update

Problem

A user adds a placeholder api_key to enable model discovery for their local server:

providers:
  loader:
    base_url: http://10.1.1.20:1234/v1
    name: loader
    api_key: no-key-required
    default_model: Qwen_V3.6_27B_dflash_c256_g0_bee

This works — the /model picker shows all models from /v1/models. But the next hermes update triggers a config migration that runs:

if old_key and old_key not in {"no-key", "no-key-required", ""}:
    new_entry["api_key"] = old_key

The api_key is silently stripped. After restart, the picker reverts to showing only default_model.

Fix

Stop filtering out "no-key" and "no-key-required". Only skip empty strings:

if old_key and old_key not in {"",}:
    new_entry["api_key"] = old_key

If a user explicitly configured an api_key — even a placeholder — the migration should preserve it.

Relationship to #29575

This PR addresses the same underlying problem from a different angle. #29575 fixes the discovery logic so providers without an api_key can still be probed. This PR fixes the migration so placeholder keys aren't silently stripped. Both fixes are independently valid; together they fully resolve the issue.

Test plan

  • Add api_key: no-key-required to a provider entry
  • Run config migration — verify the api_key is preserved in the migrated providers: dict entry
  • Verify providers with real API keys are still migrated correctly

🤖 Generated with Claude Code

…icated servers

The v11→v12 config migration (custom_providers list → providers dict)
stripped api_key values matching "no-key" or "no-key-required" from
migrated entries. These are legitimate values used by self-hosted
backends (llama.cpp, Ollama, vLLM, etc.) that expose an OpenAI-compatible
/v1/models endpoint but don't require authentication.

Without the api_key, model discovery is skipped entirely (see
model_switch.py list_authenticated_providers Section 3), causing the
/model picker to only show the single default_model instead of the
full catalog. Users who add a placeholder api_key to work around this
lose it on every config migration triggered by an update.

Stop filtering out these values — only skip empty strings. If a user
explicitly configured an api_key (even a placeholder), the migration
should preserve it.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists area/config Config system, migrations, profiles comp/cli CLI entry point, hermes_cli/, setup wizard labels May 21, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for identifying the legacy migration path. The reported model-picker behavior is already addressed on current main, but retaining placeholder strings remains a separate config-UX decision.

Problems

  • hermes_cli/model_switch.py:2088-2090 now probes keyless endpoints when no explicit models: list exists; tests/hermes_cli/test_user_providers_model_switch.py:1104-1146 covers the resulting catalog discovery.
  • The migration code has moved: current main constructs the entry through _custom_provider_entry_to_provider_config() and removes placeholder keys at hermes_cli/config.py:5592-5601, so this patch does not apply to the active implementation.
  • No migration regression test accompanies the change; existing v11→v12 coverage in tests/hermes_cli/test_config.py:1063-1104 only checks a normal API key.

Suggested changes

  • If preserving the literal placeholders is still desired, retarget hermes_cli/config.py:5600-5601 and add focused migration coverage for both placeholder values and an empty key.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users 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