fix(cli): resolve aggregator models via custom_providers and live /v1/models when models.dev is stale - #33716
Conversation
|
The aggregator fallback logic looks reasonable — checking
|
Address PR NousResearch#33716 review feedback: 1. Add tests for the new aggregator fallback code paths: - custom_providers model dict iteration resolves models not in models.dev - live fetch_endpoint_model_metadata fallback resolves missing models - live API exceptions are silently swallowed (except Exception: pass) - canonical model casing from live endpoint is preserved 2. Fix custom: prefix slug matching: - target_provider='Custom:OpenRouter' now matches entry_name='OpenRouter' - Added split(':',1)[-1].lower() suffix comparison as 5th condition - Normalise always lowercases so 'custom:openrouter' won't match 'OpenRouter' by name alone; the suffix strip handles arbitrary casing. Closes: NousResearch#33716 review feedback (liuhao1024)
|
Both observations addressed in fd00a651e:
|
Address PR NousResearch#33716 review feedback: 1. Add tests for the new aggregator fallback code paths: - custom_providers model dict iteration resolves models not in models.dev - live fetch_endpoint_model_metadata fallback resolves missing models - live API exceptions are silently swallowed (except Exception: pass) - canonical model casing from live endpoint is preserved 2. Fix custom: prefix slug matching: - target_provider='Custom:OpenRouter' now matches entry_name='OpenRouter' - Added split(':',1)[-1].lower() suffix comparison as 5th condition - Normalise always lowercases so 'custom:openrouter' won't match 'OpenRouter' by name alone; the suffix strip handles arbitrary casing. Closes: NousResearch#33716 review feedback (liuhao1024)
…oxes * fix: add tests for aggregator fallback + fix custom: prefix matching Address PR NousResearch#33716 review feedback: 1. Add tests for the new aggregator fallback code paths: - custom_providers model dict iteration resolves models not in models.dev - live fetch_endpoint_model_metadata fallback resolves missing models - live API exceptions are silently swallowed (except Exception: pass) - canonical model casing from live endpoint is preserved 2. Fix custom: prefix slug matching: - target_provider='Custom:OpenRouter' now matches entry_name='OpenRouter' - Added split(':',1)[-1].lower() suffix comparison as 5th condition - Normalise always lowercases so 'custom:openrouter' won't match 'OpenRouter' by name alone; the suffix strip handles arbitrary casing. Closes: NousResearch#33716 review feedback (liuhao1024) * fix: add HERMES_SKIP_PROFILE_OVERRIDE escape hatch for launcher sandboxes MeshBoard's stream-tap launcher creates a per-dispatch HERMES_HOME sandbox whose .env points at a local loopback proxy. Hermes' _apply_profile_override() was reading ~/.hermes/active_profile and clobbering the sandbox path, causing every dispatch to bypass the tap. Add an early return when HERMES_SKIP_PROFILE_OVERRIDE=1 is set in the environment. The MeshBoard launcher will set this alongside HERMES_HOME so the sandbox is honoured verbatim. Refs meshboard task: hermes-stream-tap-profile-override --------- Co-authored-by: Omar B <omar@kostudios.io>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
fd00a65 to
9eee472
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating the stale-catalog case and incorporating the prior discussion's request for regression coverage. The current main path still needs a live-endpoint fallback: hermes_cli/model_switch.py:1060 only checks list_provider_models(), which is models.dev-backed (agent/models_dev.py:511), before falling through to provider detection at hermes_cli/model_switch.py:1138.
Problems
- The new
custom_providersscan inhermes_cli/model_switch.py:1081accepts a model from any configured endpoint, without establishing that the entry is the active provider or shares its endpoint. That can leave a user on an aggregator that does not serve the selected model. - The added fallback tests assert only successful validation after mocking
detect_provider_for_modelto returnNoneand validation to accept. They would pass with the fallback removed, so they do not cover the intended anti-hijack guarantee.
Suggested changes
- Restrict the config match to the active provider's canonical slug or normalized base URL, and add an unrelated-custom-provider negative test.
- Make the regression tests force the Xiaomi detection fallback absent the new code, then assert the final provider remains OpenCode and the live probe was used.
Automated hermes-sweeper review.
| # user's configured aggregators. | ||
| if not resolved_in_current_catalog: | ||
| new_model_lower = new_model.lower() | ||
| # Check custom_providers model dicts |
There was a problem hiding this comment.
This accepts a model declared by any custom provider, not necessarily the active aggregator. Scope the scan to an entry whose canonical slug or normalized base URL identifies target_provider/the current endpoint; otherwise an unrelated endpoint can suppress provider detection and leave the switch on an aggregator that does not serve the model.
Why
When a user types
/model mimo-v2.5on opencode-zen (or any aggregator), the model switch pipeline falls through todetect_provider_for_model()which finds the model in xiaomi's static catalog and switches providers — requiring an API key the user doesn't have. The user gets a 401 auth error instead of staying on their configured aggregator.Root cause chain:
/model mimo-v2.5triggersswitch_model()step d (aggregator catalog search)list_provider_models('opencode')queries models.dev cache which hasmimo-v2.5-free(stale/free-tier slug) — no match for baremimo-v2.5resolved_in_current_catalogstays Falsedetect_provider_for_model('mimo-v2.5', 'opencode')finds it in xiaomi static catalogXIAOMI_API_KEY→ 401 → fallback chain → abortWhat changed
hermes_cli/model_switch.pyAggregator catalog fallback (step d): After models.dev catalog search fails to match, added two additional checks:
custom_providersmodel dicts from config.yaml — if the model is listed there, it's authoritative for the user's configured aggregator/v1/modelsendpoint viafetch_endpoint_model_metadata()— the live API returns current model IDs (e.g.mimo-v2.5) not stale models.dev slugs (e.g.mimo-v2.5-free)Validation override matching: Fixed the
custom_providersoverride in the validation rejection handler to also matchentry_name == target_providerandentry_name == normalized_target(e.g. bothopencode-zenandopencode), not justcustom:<name>. Built-in provider aliases that also appear incustom_providerswere invisible to the old check.Import: Added
normalize_providerto the imports fromhermes_cli.providers.How to review
hermes_cli/model_switch.py— two logical changes, both defensivebase_urlEvidence
Tests: 36 passed (1 pre-existing fixture failure unrelated to change).
Verification
/model mimo-v2.5on opencode-zen stays on opencode-zen (no xiaomi fallback)/model deepseek-v4-flashon opencode-go still works (existing pattern)/model kimi-k2.6on opencode-zen resolves correctlyRisks & gaps
fetch_endpoint_model_metadata()which has its own TTL cache.Note for reviewers
This is a resubmission of #33695 with only the focused model_switch.py fix (per reviewer feedback to split bundled changes). The unrelated changes (skin_engine.py, utils.py, test_atomic_replace_symlinks.py, agent_init.py) have been removed and will be submitted as separate PRs.