fix(model-switch): handle list-of-dicts models in providers config (500 on /api/model/options) - #57888
Closed
yobo2u wants to merge 1 commit into
Closed
fix(model-switch): handle list-of-dicts models in providers config (500 on /api/model/options)#57888yobo2u wants to merge 1 commit into
yobo2u wants to merge 1 commit into
Conversation
…section
list_authenticated_providers() Section 3 handled 'providers.<name>.models'
as either a dict-keyed-by-id (Hermes writer format) or a list, but the
list branch appended items verbatim. When a user configures models as a
list of dicts like:
providers:
my-provider:
models:
- id: model-a
context_length: 128000
- id: model-b
the whole dict objects were appended to the row's 'models' list. When
that row later reached hermes_cli/inventory.py:188's
user_models.update(m.lower() for m in (row.get("models") or []))
Python raised AttributeError: 'dict' object has no attribute 'lower',
bubbling out of build_models_payload() and turning /api/model/options
into a 500 that displays in the Desktop GUI as:
Failed to list model options
This only triggered when the offending provider *also* skipped the
Section 3 live /v1/models probe (should_probe=False), which happens
when the row has no api_key AND has explicit models. Providers with a
key masked the bug because fetch_api_models() overwrote the poisoned
list with a clean string list.
Fix mirrors the pattern used in the same function's Section 4 and in
_normalize_custom_provider_entry(): extract m['id'] when m is a dict,
otherwise use m verbatim. Restores the full picker payload — verified
locally against a config with volcengine-agent-plan + github-copilot
(list-of-dicts, no COPILOT/GH token) + openai-codex, which previously
500'd and now returns all 42 provider rows correctly.
Collaborator
Related: this fixes the same |
Contributor
|
Thanks for the focused reproduction and patch. This is an automated hermes-sweeper review; the behavioral fix is already present on current
The supplied triage comment correctly identified overlapping fixes; this PR's target hunk has since been superseded. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug:
providers.<name>.modelslist-of-dicts format crashes/api/model/optionswith 500Summary
When
config.yamlhas anyproviders.<name>entry whosemodels:is a list of dicts (e.g.[{id: foo, context_length: 128000}, ...]— a Hermes-supported format), and that provider falls through to Section 3 oflist_authenticated_providers()without a live-probe, the Desktop GUI's model picker fails to open with:Because
/api/model/optionsis all-or-nothing, a single misbehaving provider row poisons the entire response — even providers the user actively uses can't be listed.Environment
528159f7a(main, PR feat(skills): add security/unbroker (autonomous data-broker removal) #57438 merged)~/.hermes/hermes-agent/venvReproduce
Minimal
~/.hermes/config.yaml:Then either:
curl http://127.0.0.1:<port>/api/model/options→ HTTP 500Root Cause
hermes_cli/model_switch.py,list_authenticated_providers()Section 3, lines 1966–1969:When
cfg_modelsis[{"id": "claude-opus-4.7", "context_length": 200000}, ...], the loop appends the dict tomodels_list. That poisoned list is returned as the row's"models"field and propagates tohermes_cli/inventory.py:188:Which bubbles out of
build_models_payload()→web_server.py:4232→ the 500 response.Full stack trace (from
~/.hermes/logs/errors.log)Why This Only Bites Sometimes
Section 3 has an escape hatch that masks the bug for providers with credentials:
So a volcengine-agent-plan with an api_key hits
/v1/models, gets a clean string list, and the dict-poisoning is invisible. But a credential-less provider with an explicitmodels:list (e.g. aproviders.github-copilotentry configured for Claude Code but lackingGH_TOKEN/COPILOT_GITHUB_TOKEN) skips the probe and ships the poisoned row downstream — 500-ing the whole endpoint.Related: Section 4 (
custom_providers) handles the same shape correctly:And
_normalize_custom_provider_entry()inhermes_cli/config.py:4582-4589already knows list-of-dicts is a valid input… but silently drops them:Fix
Apply the same dict-flattening pattern already used elsewhere in the codebase (e.g.
hermes_cli/config.pysimilar sites):Verification
Before fix:
After fix:
All configured providers (
volcengine-agent-plan: 12 models,openai-codex: 4 models,github-copilot/Claude Code: 4 models) now appear in the GUI picker.Suggested Additional Improvements (out of scope for this PR)
_normalize_custom_provider_entry()inhermes_cli/config.py:4579-4589should also accept list-of-dicts (extractidfield) instead of silently dropping them./api/model/optionsshould degrade gracefully — one malformed provider shouldn't 500 the whole endpoint. Consider per-row try/except so bad rows are skipped with a warning rather than killing the entire response.hermes config check/ doctor) should warn when it sees list-of-dicts models with noidfield, or when aproviders.<name>shape doesn't match any documented schema.