diff --git a/static/ui.js b/static/ui.js index 9a3c80d7..3a42d2d7 100644 --- a/static/ui.js +++ b/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}; } diff --git a/tests/test_chat_start_provider_fallback.py b/tests/test_chat_start_provider_fallback.py index d3c6b202..181627ba 100644 --- a/tests/test_chat_start_provider_fallback.py +++ b/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") diff --git a/tests/test_issue5567_model_provider_contamination.py b/tests/test_issue5567_model_provider_contamination.py new file mode 100644 index 00000000..84293b85 --- /dev/null +++ b/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 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