fix: /api/models/live skips probe for custom providers with model config (#3718)

[DanielMaly]

Problem

/api/models/live skips the live /v1/models probe for custom and custom:* providers when the provider has a model: field in custom_providers config.

Root cause: config-specified model IDs were added directly to the ids list before the if not ids: guard that controls the live fetch. So a provider with model: assistant would set ids = ["assistant"], the guard would evaluate to False, and the upstream probe never ran.

The Settings refresh button returned only the config entry instead of the full upstream catalog.

Fix

• Collect config-specified model IDs in a separate _config_ids list instead of the main ids list
• Always attempt the live fetch for custom providers (remove the if not ids guard)
• After the fetch: merge config entries as fallback (live takes priority); if the fetch fails, use config entries as the full list

The static /api/models endpoint in config.py already handled this correctly — only the live handler had this bug.

Testing

• New regression test: tests/test_issue3718_live_models_custom_probe.py (3 assertions)
• All 33 existing + new tests pass
• Manual verification: /api/models/live?provider=custom:litellm now returns full model list (59 models) instead of just the config entry

Closes #3718

[greptile-apps]

Greptile Summary

This PR fixes a bug in _handle_live_models where custom providers configured with a model: field in custom_providers would skip the upstream /v1/models probe entirely, causing the refresh button to return only the single config entry instead of the full live catalog.

• api/routes.py: Config model IDs are now collected in a separate _config_ids list; the if not ids guard is removed so the upstream probe always runs; after the fetch, live results take priority and config entries are appended as fallback; the hardcoded timeout=8 is replaced with the named CUSTOM_MODELS_ENDPOINT_TIMEOUT_SECONDS constant (5.0 s) already defined in config.py.
• tests/test_issue3718_live_models_custom_probe.py: Six new regression tests covering the merge path, fallback-on-error, deduplication, structural guard (no if not ids and guard), a mock-based integration exercise, and the timeout-constant invariant.

Confidence Score: 5/5

Safe to merge — the production logic change is minimal, well-scoped, and correctly handles all edge cases (live success, live failure, missing credentials).

The two-variable split (ids vs _config_ids) is the smallest possible change to fix the guard logic, the merge/fallback mirrors the pattern already used by the static /api/models endpoint in config.py, and the constant import is additive. No pre-existing callers are affected.

No files require special attention.

Important Files Changed

Filename	Overview
api/routes.py	Core fix: separates config-specified model IDs into _config_ids, removes the if not ids guard to always probe upstream, and merges/falls back correctly; timeout replaced with the named constant from config
tests/test_issue3718_live_models_custom_probe.py	New regression test file with 6 tests covering merge, fallback, dedup, structural guard, mock-based integration, and timeout-constant usage; tests 1-3 verify locally-replicated logic rather than calling the production handler, but cover the right contract cases
CHANGELOG.md	Adds a clear, accurate changelog entry for the #3718 fix describing the previous mis-behavior and the new merge/fallback approach

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["/api/models/live?provider=custom:*"] --> B["provider_model_ids() → []"]
    B --> C["_config_ids = []<br/>collect model IDs from custom_providers config"]
    C --> D{"provider == 'custom'<br/>or starts with 'custom:'?"}
    D -- No --> E["Other provider flow"]
    D -- Yes --> F{"_base_url AND<br/>_api_key available?"}
    F -- No --> G["ids stays []"]
    F -- Yes --> H["HTTP GET /v1/models\ntimeout=CUSTOM_MODELS_ENDPOINT_TIMEOUT_SECONDS"]
    H -- Success --> I["ids = parsed live model list"]
    H -- Exception --> G
    I --> J{"ids non-empty?"}
    G --> J
    J -- Yes (live succeeded) --> K["Append _config_ids entries<br/>NOT already in live set"]
    J -- No (live failed / no creds) --> L["ids = list(_config_ids)"]
    K --> M["Return merged model list"]
    L --> M

Loading

_{Reviews (4): Last reviewed commit: "fix: address maintainer review feedback ..." | Re-trigger Greptile}

[nesquena-hermes]

Reading api/routes.py:10169-10300 at the PR HEAD against origin/master, plus the test file and the static probe path in api/config.py, the core fix is correct and well-scoped.

Verification

The whole change lives inside the if not ids: block at api/routes.py:10169. On entry, ids is whatever provider_model_ids(provider) returned (line 10164), which is [] for custom/custom:* since those aren't real hermes_cli endpoints. So splitting config IDs into _config_ids and dropping the if not ids and ... guard can't clobber a populated ids — there's nothing to clobber. The merge tail is a clean set-based dedup:

if ids:
    _live_set = set(ids)
    for _cid in _config_ids:
        if _cid not in _live_set:
            ids.append(_cid)
else:
    ids = list(_config_ids)

I confirmed the PR's claim that the static /api/models path "already handles this correctly": api/config.py:_read_custom_endpoint_models (around line 3915) always probes _models_endpoint_for_base_url(base) regardless of config entries. So this change brings the live handler into line with the static handler. Good.

Two divergences from the hardened static path

Now that the live fetch runs for every custom provider (not just ones without config entries), it's worth noting the live path doesn't reuse the static path's protections:

1. No SSRF guard. api/routes.py:10273 does a raw urllib.request.urlopen(_req, timeout=8) with no scheme check and no private-IP/loopback resolution guard. The static path (_read_custom_endpoint_models, api/config.py:~3920) validates the scheme and runs socket.getaddrinfo + ipaddress.is_private/is_loopback/is_link_local before fetching. The base_url here is user-configured (so risk is low), but this fix widens how often the unguarded path runs.

2. Timeout cap mismatch. The live path hardcodes timeout=8, but api/config.py:91 defines CUSTOM_MODELS_ENDPOINT_TIMEOUT_SECONDS = 5.0 with the explicit comment "Keep custom provider /v1/models probes below the frontend's generic request timeout." Since this probe now runs more often, the 8s vs 5s gap is more likely to be felt by the Settings refresh button on a slow/unreachable upstream.

Neither blocks the fix — both are pre-existing — but a follow-up that routes the live handler through _read_custom_endpoint_models (or at least shares the timeout constant and SSRF guard) would close the gap and delete the duplicated fetch/parse logic.

On the tests

The three behavioral tests reconstruct the merge/fallback algorithm inline rather than invoking _handle_live_models, so they'd keep passing even if the real handler regressed — they assert the algorithm, not the call site. The fourth test (test_live_fetch_guard_no_longer_checks_ids) is the only one tied to the actual source, via a text-grep for the marker comment. As greptile noted, consider one test that mocks urllib.request.urlopen and calls the real handler so the assertions track the code path, not a copy of it. Not a merge blocker given the structural guard, but it's where the regression coverage is thin.

[DanielMaly]

Thanks for the review! Pushed a follow-up commit addressing the two actionable points:

Timeout mismatch — Replaced the hardcoded timeout=8 with CUSTOM_MODELS_ENDPOINT_TIMEOUT_SECONDS (imported from api/config), so the live path now uses the same 5s cap as the static path. This also means any future change to the constant automatically applies here.

Test coverage — Added a mock-based integration test (test_mocked_live_fetch_returns_full_catalog_plus_config_entry) that patches urllib.request.urlopen with a fake /v1/models payload and exercises the real fetch → parse → merge path. Also added a test_timeout_uses_config_constant structural guard. The three pure-logic behavioral tests remain as they are (they verify the merge algorithm in isolation).

SSRF guard — Agree this is a pre-existing gap worth closing. I'll file a follow-up issue for routing the live handler through _read_custom_endpoint_models (or at least sharing the SSRF guard), which would also eliminate the duplicated fetch/parse logic entirely. Not in scope for this PR since it touches code well outside the bugfix.

[nesquena-hermes]

Thanks @(author) — this fix shipped in v0.51.298. Your change was cherry-picked onto the release stage during the 2026-06-06 sweep (live model probe for custom providers with model config — stage-3719), so this PR is now redundant against master (it shows as conflicting because the fix is already present).

Closing as indirectly merged with full attribution. Appreciate the contribution! 🙏

[nesquena-hermes]

(Correcting attribution above: thanks @DanielMaly for this — credited in the release CHANGELOG.)