Skip to content

fix: clear stale api_key when switching custom providers - #8571

Open
yiminglin-ai wants to merge 1 commit into
NousResearch:mainfrom
yiminglin-ai:fix/clear-stale-custom-provider-api-key
Open

fix: clear stale api_key when switching custom providers#8571
yiminglin-ai wants to merge 1 commit into
NousResearch:mainfrom
yiminglin-ai:fix/clear-stale-custom-provider-api-key

Conversation

@yiminglin-ai

Copy link
Copy Markdown

Summary

  • clear model.api_key when switching to a saved custom provider that does not define an API key
  • keep the existing api_mode behavior intact
  • extend the named custom-provider regression test to cover stale auth cleanup

Problem

_model_flow_named_custom() already clears stale api_mode when the selected custom_providers entry does not specify one, but it leaves a stale model.api_key behind.

That means switching from an authenticated custom endpoint to a no-key local endpoint can preserve the old key in config.yaml, which is incorrect state and can affect subsequent requests.

Testing

  • /Users/yiminglin/.hermes/hermes-agent/venv/bin/pytest -q tests/hermes_cli/test_custom_provider_model_switch.py
  • manual temp-HERMES_HOME reproducer showing model.api_key is removed when selecting a no-key provider

@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 28, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused regression fix. The underlying legacy named-custom-provider case remains valid on current main.

Problems

  • The implementation hunk is stale because _model_flow_named_custom moved from hermes_cli/main.py to hermes_cli/model_setup_flows.py in a77efada5f55436e6a17da45a30a3352ce24a780. Current hermes_cli/model_setup_flows.py:1515-1516 still writes model.api_key only when a key exists, leaving a prior key intact otherwise.

Suggested changes

  • During salvage, relocate the else: model.pop("api_key", None) immediately after hermes_cli/model_setup_flows.py:1515-1516 and carry over the stale-key regression assertion. The provider-key path already clears the field at hermes_cli/model_setup_flows.py:1506-1509.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data 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

@GottZ GottZ left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This was generated by AI during triage.

Summary

Four PRs address or reference stale endpoint credentials during provider switches. #8571 covers the remaining named-custom-to-no-key-custom path, while #49360, #49362, and merged #49380 address switching away from custom providers, with #49380 implementing the broad shared-helper solution.

Related pull requests

  • #8571 related — (+8/-4) — salvage and merge: the diff correctly removes a stale model.api_key when a selected named custom provider has no key and adds a focused regression assertion. The contributor keep_open review confirms that the bug remains on current main, but the implementation and test must be relocated from hermes_cli/main.py to the current model_setup_flows.py path.
  • #49360 [closed] related — (+260/-17) — merged via #49380: this comprehensive shared-helper implementation clears api_key, legacy api, and api_mode across CLI, web, gateway, and auxiliary provider-switch paths. It remains relevant as the source implementation cherry-picked with preserved authorship into the merged consolidation PR.
  • #49362 [closed] related — (+240/-0) — superseded by #49380: it fixes the stale in-memory Nous rewrite plus OpenRouter and generic API-key-provider paths, but #49380 covers the same cases and the wider provider-switch class through a shared helper.
  • #49380 [merged] related — (+260/-17) — merged reference implementation: it centralizes endpoint-credential cleanup and applies it across approximately thirteen non-custom provider-switch paths, including the stale Nous in-memory-config case. Its diff does not add the named-custom-provider no-key cleanup covered by #8571.

Duplicates

#49360 and #49380 are materially the same comprehensive implementation because #49360 was cherry-picked into #49380; #49362 overlaps their Nous, OpenRouter, and generic API-key-provider subset. #8571 is not a duplicate because it covers the distinct named-custom-to-no-key-custom transition.

Suggested consolidation

Salvage and merge #8571 after relocating its implementation and regression test to the current model_setup_flows.py structure, completing the named-custom-provider gap left by merged #49380. Keep #49360 and #49362 closed as merged-via/superseded-by #49380; no reopening is warranted.

Cross-PR triage: Reviewed 4 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 58 kB of PR diffs, 11 kB of issue/PR text, 5 kB of discussion (6 comments), 1 verify verdict. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

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-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.

4 participants