fix(#5567): resolve model provider by matching value, not the selected option

Frontend fix for the model-picker provider-contamination vector of #5567 (the
backend HERMES_HOME vector shipped in #5577). Credit @b3nw for the root-cause trace.

_modelStateForSelect(sel, modelId) resolved the provider from sel.selectedOptions[0]
(the dropdown's CURRENTLY-selected option) instead of the option whose value matches
modelId. During a profile/tab switch or a model-list rebuild the dropdown transiently
still holds the PREVIOUS profile's default option (e.g. an ollama model), so the wrong
provider was stamped onto a model it doesn't own, persisted into the session's
model_provider, and (since _modelProviderForSend reads the stored value first) re-sent
on every turn -> 'Provider ollama...no API key' for a provider the session never used.

Fix: resolve the provider from the option whose VALUE === modelId; trust the
currently-selected option only when its value matches (preserving the user's exact
pick in the same-value/different-provider collision case). A genuinely-absent model
yields a null provider so the backend re-infers.

This is the WRITE-site fix — it stops all NEW contamination (new-session + send paths
route through _modelStateForSelect). Both Codex and Fable reviewed it SAFE.

Repairing sessions ALREADY poisoned before this shipped is deliberately deferred to a
separate follow-up: a 3-round Codex+Fable gate proved a frontend load-time repair that
reads the shared #modelSelect DOM is a cross-session race surface (it re-manufactured
the reverse-contamination it aimed to fix), and it can't reliably heal live-only-model
sessions anyway. The robust place for that repair is the backend chat-start boundary,
where session + profile identity is unambiguous. Already-poisoned sessions self-heal on
their next healthy send / manual re-pick in the meantime.

Tests: node-behavioral resolver scenarios (tab-switch race, same-value collision,
missing-model null, match-by-value, default->null) + the unrelated-selected-option
case from #5727. Partially addresses #5567 (write-vector); follow-up tracks the
already-poisoned-session repair.

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

static/ui.js

@@ -2570,7 +2570,24 @@ function _modelStateForSelect(sel, modelId){
   if(!value) return {model:'',model_provider:null};
   const explicitProvider=_providerFromModelValue(value);
   if(explicitProvider) return {model:value,model_provider:explicitProvider};
-  const opt=sel&&sel.selectedOptions&&sel.selectedOptions[0];
+  // Resolve the provider from the option whose VALUE matches the requested
+  // model — never blindly from sel.selectedOptions[0] (#5567). During a profile
+  // /tab switch or a model-list rebuild the dropdown transiently still has the
+  // PREVIOUS profile's default option selected (e.g. an ollama model), so reading
+  // selectedOptions[0] would stamp that foreign provider onto a model it doesn't
+  // own — which is then persisted into the session's model_provider and re-sent
+  // on every turn, bricking it with a "Provider 'X'…no API key" error for a
+  // provider the session never used.
+  let opt=null;
+  const selected=sel&&sel.selectedOptions&&sel.selectedOptions[0];
+  // Prefer the currently-selected option ONLY when it actually is the requested
+  // model — this preserves the user's exact pick in the same-value/different-
+  // provider collision case (two providers offering the same model id).
+  if(selected&&String(selected.value||'')===value){
+    opt=selected;
+  }else if(sel&&sel.options){
+    opt=Array.from(sel.options).find(o=>String(o.value||'')===value)||null;
+  }
   const provider=String(_getOptionProviderId(opt)||'').trim();
   return {model:value,model_provider:(provider&&provider!=='default')?provider:null};
 }

tests/test_chat_start_provider_fallback.py

@@ -215,6 +215,24 @@ def test_model_state_reads_live_custom_provider_from_option_metadata(driver_path
     assert state == {"model": "llama-3.3-custom", "model_provider": "custom:lab"}
 
 
+@node_test
+def test_model_state_does_not_use_unrelated_selected_option_provider(driver_path):
+    # If the requested model is not in the options and differs from the currently
+    # selected option, it should return model_provider = null instead of the selected option's provider.
+    state = _run_model_state_helper(driver_path, {
+        "model": "kilo/minimax/minimax-m3",
+        "initialValue": "qwen3.6:27b-mlx",
+        "options": [{
+            "provider": "ollama",
+            "optionProvider": "ollama",
+            "value": "qwen3.6:27b-mlx",
+        }],
+    })
+
+    assert state == {"model": "kilo/minimax/minimax-m3", "model_provider": None}
+
+
+
 def test_live_custom_models_are_tagged_with_provider_metadata():
     ui_src = UI_JS_PATH.read_text(encoding="utf-8")
     start = ui_src.index("function _addLiveModelsToSelect")

tests/test_issue5567_model_provider_contamination.py

@@ -0,0 +1,174 @@
+"""Regression coverage for #5567 (frontend vector): model_provider contamination.
+
+Two distinct bugs produce the identical "Provider 'ollama'…no API key" symptom:
+
+  * #5577 (shipped) — backend HERMES_HOME clobber → init reads a FOREIGN
+    profile's config.yaml. Fixed at the agent reader (context-local home override).
+  * THIS one — the frontend resolver `_modelStateForSelect` read
+    `sel.selectedOptions[0]` (the DOM's currently-selected option) instead of the
+    option whose value matches the requested model. During a profile/tab switch
+    the dropdown transiently still has the PREVIOUS profile's default selected
+    (e.g. an ollama model), so a send in that window stamps `ollama` onto a model
+    it doesn't own. That wrong provider is persisted into the session JSON
+    (`model_provider`) and, because `_modelProviderForSend` reads the stored value
+    FIRST, re-sent on every subsequent turn — a sticky brick.
+
+This ships the RESOLVER fix (stops all NEW contamination at the write site). A
+separate follow-up (tracked issue) will repair sessions ALREADY poisoned before
+this shipped, done at the backend chat-start boundary where session + profile
+identity is unambiguous — NOT via the shared frontend #modelSelect DOM, which a
+three-round gate proved is a cross-session race surface that re-manufactures the
+very contamination it tries to fix.
+
+This suite exercises the JS in Node with a mock <select> so it fails without the
+fix and passes with it.
+"""
+
+import json
+import shutil
+import subprocess
+from pathlib import Path
+
+import pytest
+
+
+ROOT = Path(__file__).resolve().parents[1]
+UI_JS = ROOT / "static" / "ui.js"
+NODE = shutil.which("node")
+
+
+def _read(path: Path) -> str:
+    return path.read_text(encoding="utf-8")
+
+
+# ---------------------------------------------------------------------------
+# Source-level guards (fast, no node) — lock the shape of the fix in place.
+# ---------------------------------------------------------------------------
+
+def test_model_state_no_longer_blindly_reads_selected_option():
+    """The bug was `const opt=sel&&sel.selectedOptions&&sel.selectedOptions[0];`
+    used unconditionally as the provider source. That exact unconditional read
+    must be gone; the resolver must match the option by value."""
+    src = _read(UI_JS)
+    start = src.index("function _modelStateForSelect(sel, modelId)")
+    body = src[start : src.index("function _captureModelDropdownSelection", start)]
+    # It must resolve by matching option value.
+    assert "Array.from(sel.options).find(o=>String(o.value||'')===value)" in body
+    # It may still prefer the selected option, but ONLY when it matches the value.
+    assert "selected&&String(selected.value||'')===value" in body
+    # The unconditional "trust selectedOptions[0]" read must not survive.
+    assert "const opt=sel&&sel.selectedOptions&&sel.selectedOptions[0];" not in body
+
+
+# ---------------------------------------------------------------------------
+# Behavioral test in Node — the real repro the maintainer asked for.
+# ---------------------------------------------------------------------------
+
+_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 depth = 0;
+  for (let i = brace; i < source.length; i++) {
+    if (source[i] === '{') depth += 1;
+    else if (source[i] === '}') { depth -= 1; if (depth === 0) return source.slice(start, i + 1); }
+  }
+  throw new Error('unterminated: ' + name);
+}
+
+// Dependencies pulled in verbatim from ui.js so we test the real code.
+eval(extractFunction(uiSrc, '_getOptionProviderId'));
+eval(extractFunction(uiSrc, '_providerFromModelValue'));
+eval(extractFunction(uiSrc, '_modelStateForSelect'));
+
+// Minimal mock <select> option: dataset carries the provider (as the real
+// optgroup/option markup does via data-provider).
+function opt(value, provider) {
+  return { value: value, dataset: provider ? { provider: provider } : {}, parentElement: null };
+}
+
+const results = {};
+
+// --- Scenario 1: the tab-switch race. Dropdown still has the PREVIOUS profile's
+//     ollama default selected, but we're resolving a kilo/* model. Provider must
+//     NOT come out as ollama.
+{
+  const ollamaOpt = opt('qwen3.6:27b-mlx', 'ollama');
+  const kiloOpt = opt('kilo/minimax/minimax-m3', 'kilocode');
+  const sel = { options: [ollamaOpt, kiloOpt], selectedOptions: [ollamaOpt] };
+  results.race = _modelStateForSelect(sel, 'kilo/minimax/minimax-m3');
+}
+
+// --- Scenario 2: same-value/different-provider collision. The user explicitly
+//     selected the option (selectedOptions[0].value === requested value); that
+//     exact pick must be preserved.
+{
+  const a = opt('shared-model', 'provider-a');
+  const b = opt('shared-model', 'provider-b');
+  const sel = { options: [a, b], selectedOptions: [b] };
+  results.collision = _modelStateForSelect(sel, 'shared-model');
+}
+
+// --- Scenario 3: model not present in the (rebuilt) dropdown at all → null
+//     provider, never the stale selected one.
+{
+  const stale = opt('qwen3.6:27b-mlx', 'ollama');
+  const sel = { options: [stale], selectedOptions: [stale] };
+  results.missing = _modelStateForSelect(sel, 'kilo/minimax/minimax-m3');
+}
+
+// --- Scenario 4: the resolver picks the matching option's provider even when a
+//     DIFFERENT option is currently selected (the general form of the fix).
+{
+  const selectedOther = opt('gpt-4o', 'openai');
+  const wanted = opt('claude-opus-4.6', 'anthropic');
+  const sel = { options: [selectedOther, wanted], selectedOptions: [selectedOther] };
+  results.match_by_value = _modelStateForSelect(sel, 'claude-opus-4.6');
+}
+
+// --- Scenario 5: a 'default' provider attribution yields null (not 'default').
+{
+  const o = opt('some-model', 'default');
+  const sel = { options: [o], selectedOptions: [o] };
+  results.default_provider = _modelStateForSelect(sel, 'some-model');
+}
+
+process.stdout.write(JSON.stringify(results));
+"""
+
+
+@pytest.mark.skipif(NODE is None, reason="node not on PATH")
+def test_model_provider_resolution_behavior():
+    proc = subprocess.run(
+        [NODE, "-e", _DRIVER, str(UI_JS)],
+        capture_output=True,
+        text=True,
+        timeout=30,
+    )
+    assert proc.returncode == 0, f"node driver failed: {proc.stderr}"
+    r = json.loads(proc.stdout)
+
+    # Scenario 1 — the core bug: a tab-switch-race resolution must not attribute
+    # the previous profile's ollama provider to a kilo/* model.
+    assert r["race"]["model"] == "kilo/minimax/minimax-m3"
+    assert r["race"]["model_provider"] != "ollama"
+    assert r["race"]["model_provider"] == "kilocode"
+
+    # Scenario 2 — explicit same-value pick is preserved.
+    assert r["collision"]["model_provider"] == "provider-b"
+
+    # Scenario 3 — missing model resolves to a null provider (backend re-infers),
+    # never the stale selected ollama.
+    assert r["missing"]["model_provider"] is None
+
+    # Scenario 4 — resolver matches by value, not by which option is selected.
+    assert r["match_by_value"]["model"] == "claude-opus-4.6"
+    assert r["match_by_value"]["model_provider"] == "anthropic"
+
+    # Scenario 5 — a 'default' provider attribution collapses to null.
+    assert r["default_provider"]["model_provider"] is None