fix(models): prevent bare-id picker revert when provider hint is empty (#6195) (#6199)

Prevent bare-id model picker revert when the provider hint is empty: an ambiguous bare id that collides across provider groups no longer snaps to the default group on re-render. Adds a revert-sensitive regression test and fixes three cross-file test-isolation leaks found while gating. Thanks @webtecnica.

Co-authored-by: webtecnica <webtecnica@users.noreply.github.com>

static/ui.js

@@ -3197,9 +3197,19 @@ function _findModelInDropdown(modelId, sel, preferredProviderId){
     const providerMatch=options.find(o=>norm(o.value)===target && _getOptionProviderId(o).toLowerCase()===preferred);
     if(providerMatch) return providerMatch.value;
   }
-  // 2. Normalized match
+  // 2. Normalized match — but ONLY when unambiguous. If the bare id
+  // matches across multiple provider groups AND no provider hint is
+  // available, return null instead of snapping to the first group's
+  // option. This prevents a deliberate non-default pick from reverting
+  // to the default provider on re-render (#6195).
   const exact=opts.find(o=>norm(o)===target);
-  if(exact) return exact;
+  if(exact){
+    const normMatches=options.filter(o=>norm(o.value)===target);
+    if(normMatches.length>1 && !preferred && !explicitProvider && !rawModel.includes('/')){
+      return null;  // ambiguous bare id — caller must inject the correct option
+    }
+    return exact;
+  }
   // If the request is provider-qualified (either explicit @provider:model or
   // a slash-qualified vendor/model id), do NOT fuzzy-match a sibling model
   // once exact/provider-aware lookup failed. Returning null lets the caller

tests/test_agent_runtime_revision_guard.py

@@ -39,6 +39,23 @@ def test_loaded_agent_runtime_fails_closed_after_source_revision_changes(tmp_pat
     probe = tmp_path / "probe.py"
     probe.write_text(
         """
+import sys
+
+# On a machine where hermes_agent is installed as an editable package, setuptools
+# registers a meta-path finder (__editable___hermes_agent_*_finder) that maps
+# `run_agent` -> the real on-disk agent module. That finder runs BEFORE the
+# PYTHONPATH-based PathFinder, so it would shadow the synthetic per-test agent
+# dir this test points HERMES_WEBUI_AGENT_DIR at (the real AIAgent has no
+# `.revision` attribute). Drop any such editable finder + purge cached agent
+# modules so `import run_agent` resolves from the test's agent_dir on PYTHONPATH.
+# On CI (no editable install) this is a harmless no-op.
+sys.meta_path[:] = [
+    _f for _f in sys.meta_path
+    if "__editable__" not in type(_f).__module__ and "__editable__" not in getattr(_f, "__module__", "")
+]
+for _m in ("run_agent", "hermes_state", "agent", "tools"):
+    sys.modules.pop(_m, None)
+
 from pathlib import Path
 import subprocess
 

tests/test_issue2720_bedrock_model_picker.py

@@ -22,6 +22,7 @@ def _force_env_fallback(monkeypatch):
 def _run_available_models_with_cfg(monkeypatch, tmp_path, cfg):
     old_cfg = dict(config.cfg)
     old_mtime = config._cfg_mtime
+    old_cfg_path = config._cfg_path
     monkeypatch.setattr(config, "_models_cache_path", tmp_path / "models_cache.json")
     monkeypatch.setattr(config, "_get_config_path", lambda: tmp_path / "missing-config.yaml")
     monkeypatch.setattr("api.profiles.get_active_hermes_home", lambda: tmp_path, raising=False)
@@ -35,6 +36,12 @@ def _run_available_models_with_cfg(monkeypatch, tmp_path, cfg):
         config.cfg.clear()
         config.cfg.update(old_cfg)
         config._cfg_mtime = old_mtime
+        # get_available_models() stamps config._cfg_path to the monkeypatched
+        # tmp path as a side effect of its staleness check; restore it so a
+        # later test's get_available_models() doesn't see path_changed=True and
+        # reload config from disk over that test's in-memory cfg (isolation leak
+        # that flaked test_issue3691 / test_issue1426 under cross-file ordering).
+        config._cfg_path = old_cfg_path
         config.invalidate_models_cache()
 
 

tests/test_issue6195_bare_id_ambiguous_no_revert.py

@@ -0,0 +1,109 @@
+"""Regression for #6195 / #6199: a bare model id that collides with the profile
+default across multiple provider groups must NOT silently snap to the default
+group's option when no provider hint is available.
+
+`_findModelInDropdown('glm-5.2', sel, '')` previously returned the FIRST
+normalized match (`z-ai/glm-5.2`, the default group), so a deliberate non-default
+pick reverted to the default provider on re-render when the provider hint was
+momentarily unavailable. The Stage-2 guard now returns null in that ambiguous
+case so the caller injects a provider-scoped option instead of asserting the
+wrong provider.
+
+Invariants this locks in (proven against origin/master vs the fix):
+  - ambiguous bare id + EMPTY hint  -> null   (was: default-group snap)  [FIXED]
+  - ambiguous bare id + correct hint -> the hinted provider option        [unchanged]
+  - uniquely-named bare id           -> its single match                  [unchanged]
+  - bare id that IS an exact option value -> returned at Stage 0          [unchanged]
+"""
+
+from __future__ import annotations
+
+import json
+import shutil
+import subprocess
+from pathlib import Path
+
+import pytest
+
+REPO_ROOT = Path(__file__).parent.parent.resolve()
+UI_JS = REPO_ROOT / "static" / "ui.js"
+NODE = shutil.which("node")
+
+pytestmark = pytest.mark.skipif(NODE is None, reason="node not on PATH")
+
+_DRIVER = r"""
+const fs = require('fs');
+const uiSrc = fs.readFileSync(process.argv[1], 'utf8');
+
+function extractFunction(source, name){
+  const marker='function '+name+'(';
+  const start=source.indexOf(marker);
+  if(start<0) throw new Error('not found: '+name);
+  const brace=source.indexOf('{', source.indexOf(')', start));
+  let d=0;
+  for(let i=brace;i<source.length;i++){
+    if(source[i]==='{') d++;
+    else if(source[i]==='}'){ d--; if(d===0) return source.slice(start,i+1); }
+  }
+  throw new Error('unterminated: '+name);
+}
+
+eval([
+  '_getOptionProviderId',
+  '_providerFromModelValue',
+  '_findModelInDropdown',
+].map(name => extractFunction(uiSrc, name)).join('\n'));
+
+function makeOpt(value, provider){
+  return { value, textContent:value, dataset:{}, parentElement:{tagName:'OPTGROUP', dataset:{provider}} };
+}
+function makeSelect(options){
+  let idx = 0;
+  return {
+    id:'modelSelect', options,
+    querySelectorAll(){ return []; },
+    get selectedOptions(){ return idx>=0?[options[idx]]:[]; },
+    get value(){ return idx>=0?options[idx].value:''; },
+    set value(v){ idx = options.findIndex(o=>o.value===v); },
+  };
+}
+
+const AMB = () => [ makeOpt('z-ai/glm-5.2','z-ai'), makeOpt('@custom:tokenrouter:z-ai/glm-5.2','custom:tokenrouter') ];
+
+const out = {
+  // #6195 repro: ambiguous bare id, no hint -> must be null (not a default-group snap)
+  ambiguousEmptyHint: _findModelInDropdown('glm-5.2', makeSelect(AMB()), ''),
+  // with the correct hint -> resolves to the hinted provider (unchanged)
+  ambiguousWithHint: _findModelInDropdown('glm-5.2', makeSelect(AMB()), 'custom:tokenrouter'),
+  // uniquely-named bare id still matches (unchanged)
+  uniqueBare: _findModelInDropdown('glm-5.2-free', makeSelect([makeOpt('z-ai/glm-5.2-free','z-ai')]), ''),
+  // bare id that IS an exact option value -> returned at Stage 0 even when ambiguous (unchanged)
+  bareExactAmbiguous: _findModelInDropdown('glm-5.2', makeSelect([makeOpt('glm-5.2','z-ai'), makeOpt('@custom:tokenrouter:glm-5.2','custom:tokenrouter')]), ''),
+};
+process.stdout.write(JSON.stringify(out));
+"""
+
+
+def _run():
+    assert NODE is not None
+    result = subprocess.run([NODE, "-e", _DRIVER, str(UI_JS)],
+                            capture_output=True, text=True, timeout=30)
+    assert result.returncode == 0, result.stderr
+    return json.loads(result.stdout)
+
+
+def test_ambiguous_bare_id_no_hint_does_not_snap_to_default():
+    # THE fix: no wrong-provider revert. null => caller injects a scoped option.
+    assert _run()["ambiguousEmptyHint"] is None
+
+
+def test_ambiguous_bare_id_with_hint_resolves_to_hinted_provider():
+    assert _run()["ambiguousWithHint"] == "@custom:tokenrouter:z-ai/glm-5.2"
+
+
+def test_unique_bare_id_still_matches():
+    assert _run()["uniqueBare"] == "z-ai/glm-5.2-free"
+
+
+def test_bare_id_that_is_exact_option_value_returns_at_stage0():
+    assert _run()["bareExactAmbiguous"] == "glm-5.2"

tests/test_provider_sort_order.py

@@ -48,7 +48,19 @@ def _install_fake_hermes_cli(monkeypatch):
 
 @pytest.fixture(autouse=True)
 def _isolate_cache():
-    """Invalidate the TTL model cache around each test."""
+    """Invalidate the TTL model cache around each test AND restore config.cfg.
+
+    ``_setup_config`` calls ``config.reload_config()`` against a temp config.yaml,
+    which repopulates the shared ``config.cfg`` with this file's provider/model
+    config and never resets it. Under cross-file ordering a sibling test (e.g.
+    test_model_resolver) then inherits a leaked ``providers`` block that injects
+    an unexpected provider group and flakes its assertions. Snapshot + restore
+    ``config.cfg`` (a shallow dict copy — cheap) so the leak can't escape this
+    file. We deliberately do NOT touch ``_cfg_cache`` / ``_yaml_file_cache``
+    (restoring those globals destabilizes unrelated state-db reconciliation
+    tests in the same shard).
+    """
+    _saved_cfg = dict(config.cfg)
     try:
         config.invalidate_models_cache()
     except Exception:
@@ -58,6 +70,12 @@ def _isolate_cache():
         config.invalidate_models_cache()
     except Exception:
         pass
+    try:
+        if isinstance(config.cfg, dict):
+            config.cfg.clear()
+            config.cfg.update(_saved_cfg)
+    except Exception:
+        pass
 
 
 def _model_list(*ids):