From cfdfe4db8f084d136954785185eecee00fbedbf8 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Wed, 9 Sep 2026 13:49:47 -0700 Subject: [PATCH] release: #7261 provider-native auxiliary model IDs (approved) (#7491) * fix(settings): persist provider-native auxiliary model IDs Co-authored-by: lowlandsheperd * docs(changelog): stamp #7261 provider-native aux model IDs --------- Co-authored-by: n Co-authored-by: lowlandsheperd --- CHANGELOG.md | 2 + api/config.py | 32 ++- static/panels.js | 50 ++-- tests/test_auxiliary_model_provider_prefix.py | 253 ++++++++++++++++++ tests/test_issue4813_cron_model_bare_name.py | 2 +- 5 files changed, 319 insertions(+), 20 deletions(-) create mode 100644 tests/test_auxiliary_model_provider_prefix.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 57f57f85..37e4e52b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ - **The workspace switcher now lists workspaces in your configured order instead of re-sorting them alphabetically.** The chat-header workspace dropdown alphabetized its entries client-side, so it disagreed with the drag-and-drop order shown in the Workspaces settings panel (and never kept the default "Home" workspace first). The dropdown now renders the server's stored order verbatim — the same order as the settings view — with no client-side re-sort. Thanks @CharlesMcquade. (#7317) +- **Auxiliary-model settings now persist provider-native model IDs instead of silently dropping the provider prefix.** When an auxiliary model (title/summary/etc.) was chosen as a provider-qualified ID, settings canonicalization stripped the `@provider:` prefix and could persist a bare name that resolved under the wrong provider group. The picker now preserves the provider-native ID, dedupes qualified/unqualified variants, and fails closed on an ambiguous mismatch rather than persisting a wrong upstream model. Thanks @lowlandsheperd. (#7261) + - **Hardened a streaming race that could read a stream queue mid-teardown.** Several code paths read the in-memory SSE stream registry with a bare, unlocked `STREAMS.get()`, so a read that raced a concurrent teardown (cancel/finish popping the queue under `STREAMS_LOCK`) could observe-and-use a queue the registry had already released. Registry reads now go through a lock-disciplined `peek_stream()` helper that takes `STREAMS_LOCK`, and a self-enforcing test forbids reintroducing a bare read. Callers keep their existing empty/None-guard fallbacks, so behavior on the common non-race path is unchanged. Thanks @zicochaos. (#7339) - **Automatic session titles no longer keep an internal placeholder for workspace-prefixed multimodal first turns, and internal scaffolding no longer leaks into generated titles.** Title eligibility compared persisted content that still carried WebUI workspace/attachment scaffolding against a sanitized visible title, so a workspace-prefixed structured multimodal first turn could never be recognized as provisional and stayed stuck on its placeholder title; the same internal scaffolding could also reach the title provider's input. Title generation now sanitizes title-facing structured text (stripping versioned workspace sentinels and generated attachment suffixes) across eligibility, first-exchange generation, manual prefer-latest generation, and adaptive refresh, while preserving genuine user content and native image parts. Persisted and model-facing message content is unchanged (titles only read it), and a real user-chosen title is never misclassified as provisional. Thanks @starship-s. (#7306) diff --git a/api/config.py b/api/config.py index 2301ff85..ed6b5a07 100644 --- a/api/config.py +++ b/api/config.py @@ -5013,6 +5013,31 @@ def _coerce_optional_positive_int(value, field: str): return number +def _provider_native_auxiliary_model(provider: str, model: str) -> str: + """Return the provider-native model stored by an auxiliary slot. + + ``@provider:model`` is a WebUI picker routing token. Auxiliary slots already + store the selected provider separately, so only an exact matching prefix is + safe to remove. Reject other qualified forms instead of persisting an + ambiguous upstream model name. + """ + provider_id = str(provider or "").strip() or "auto" + model_id = str(model or "").strip() + if not model_id.startswith("@") or ":" not in model_id: + return model_id + + matching_prefix = f"@{provider_id}:" + if provider_id != "auto" and model_id.startswith(matching_prefix): + native_model = model_id[len(matching_prefix) :] + if native_model: + return native_model + + raise ValueError( + "provider-qualified auxiliary model must match the selected provider " + "and include a model name" + ) + + def set_auxiliary_model(task: str, provider: str, model: str, advanced: dict | None = None) -> dict: """Persist an auxiliary model assignment in config.yaml. @@ -5021,6 +5046,8 @@ def set_auxiliary_model(task: str, provider: str, model: str, advanced: dict | N Sensitive api_key values are write-only: get_auxiliary_models() only reports whether one is set. """ + provider = str(provider or "").strip() or "auto" + model = str(model or "").strip() config_path = _get_config_path() with _cfg_lock: config_data = _load_yaml_config_file(config_path) @@ -5046,11 +5073,12 @@ def set_auxiliary_model(task: str, provider: str, model: str, advanced: dict | N aux_cfg = config_data.get("auxiliary", {}) if not isinstance(aux_cfg, dict): aux_cfg = {} + model = _provider_native_auxiliary_model(provider, model) slot_cfg = aux_cfg.get(task, {}) if not isinstance(slot_cfg, dict): slot_cfg = {} - slot_cfg["provider"] = provider or "auto" - slot_cfg["model"] = model or "" + slot_cfg["provider"] = provider + slot_cfg["model"] = model if provider and (provider.startswith("custom:") or provider == "custom"): # Resolve the auxiliary slot's base_url against the SELECTED # provider, not the active main provider. A bare diff --git a/static/panels.js b/static/panels.js index d64d9323..c33bcdce 100644 --- a/static/panels.js +++ b/static/panels.js @@ -1889,17 +1889,19 @@ function cancelCronForm(){ _clearCronDetail(); } -function _cronModelBareName(model, provider) { +function _modelBareNameForProvider(model, provider) { // Strip @provider: prefix from a model value when provider is stored separately. - // The model dropdown may contain values like "@custom:9router:chat" (from - // _apply_provider_prefix) but cron jobs store model and provider separately, - // so the model should be just "chat". if (model && provider && model.startsWith('@' + provider + ':')) { return model.slice(('@' + provider + ':').length); } return model; } +function _cronModelBareName(model, provider) { + // Cron jobs store model and provider separately, just like auxiliary slots. + return _modelBareNameForProvider(model, provider); +} + async function saveCronForm(){ const nameEl=$('cronFormName'); const schEl=$('cronFormSchedule'); @@ -12326,17 +12328,25 @@ function _buildAuxModelOptions(sel,provider,providers,currentModel){ const emptyOpt=document.createElement('option'); emptyOpt.value='';emptyOpt.textContent=t('settings_aux_model_auto')||'auto (use provider default)'; sel.appendChild(emptyOpt); + const canonicalCurrent=_modelBareNameForProvider(currentModel,provider)||''; if(!provider||provider==='auto'){ - sel.value=currentModel||''; - return; + sel.value=canonicalCurrent; + return canonicalCurrent; } // Find matching provider in cached list const pData=providers.find(p=>p.slug===provider); + const modelValues=new Set(); if(pData&&pData.models){ - for(const mId of pData.models){ + for(const modelEntry of pData.models){ + const routeId=typeof modelEntry==='string'?modelEntry:String(modelEntry?.id||''); + const mId=_modelBareNameForProvider(routeId,provider)||''; + if(!mId||modelValues.has(mId)) continue; + modelValues.add(mId); + const routeLabel=typeof modelEntry==='string'?'':String(modelEntry?.label||''); + const modelLabel=_modelBareNameForProvider(routeLabel,provider)||mId; const opt=document.createElement('option'); - opt.value=mId;opt.textContent=mId; - if(mId===currentModel) opt.selected=true; + opt.value=mId;opt.textContent=modelLabel; + if(mId===canonicalCurrent) opt.selected=true; sel.appendChild(opt); } } @@ -12345,12 +12355,13 @@ function _buildAuxModelOptions(sel,provider,providers,currentModel){ customOpt.value='__custom__';customOpt.textContent=t('settings_aux_model_custom')||'Custom model…'; sel.appendChild(customOpt); // If currentModel not in list and not empty, add it as a custom option - if(currentModel&&!pData?.models?.includes(currentModel)){ + if(canonicalCurrent&&!modelValues.has(canonicalCurrent)){ const existingOpt=document.createElement('option'); - existingOpt.value=currentModel;existingOpt.textContent=currentModel+' (configured)'; + existingOpt.value=canonicalCurrent;existingOpt.textContent=canonicalCurrent+' (configured)'; existingOpt.selected=true; sel.insertBefore(existingOpt,customOpt); } + return canonicalCurrent; } function _onAuxProviderChange(taskKey,providers){ @@ -12368,13 +12379,16 @@ async function _onAuxModelChange(taskKey){ if(modelSel.value==='__custom__'){ const customModel=await showPromptDialog({title:t('settings_aux_model_custom')||'Custom model',message:t('settings_aux_model_custom_prompt')||'Enter model ID:',placeholder:'model/provider:model-id',confirmLabel:t('settings_btn_apply_aux_models')||'Apply'}); if(customModel&&customModel.trim()){ + const provider=$('aux-prov-'+taskKey)?.value||''; + const enteredModel=customModel.trim(); + const canonicalModel=_modelBareNameForProvider(enteredModel,provider)||enteredModel; // Insert custom model option before the __custom__ option const opt=document.createElement('option'); - opt.value=customModel.trim();opt.textContent=customModel.trim(); + opt.value=canonicalModel;opt.textContent=canonicalModel; // Remove __custom__ selection const customIdx=[...modelSel.options].findIndex(o=>o.value==='__custom__'); if(customIdx>=0) modelSel.insertBefore(opt,modelSel.options[customIdx]); - modelSel.value=customModel.trim(); + modelSel.value=canonicalModel; }else{ modelSel.value=''; } @@ -12581,7 +12595,7 @@ async function _loadAuxiliaryModels(){ _auxProviders=groups.filter(g=>g.provider&&((g.models&&g.models.length>0)||(g.extra_models&&g.extra_models.length>0))).map(g=>({ slug:g.provider_id||g.provider, name:g.provider, - models:[...(g.models||[]),...(g.extra_models||[])].map(m=>m.id), + models:[...(g.models||[]),...(g.extra_models||[])].map(m=>({id:m.id,label:m.label||m.id})), })); if(auxData&&Object.prototype.hasOwnProperty.call(auxData,'main')){ _mainAdvancedConfig=auxData.main||{}; @@ -12596,6 +12610,7 @@ async function _loadAuxiliaryModels(){ _auxOriginalConfig=JSON.parse(JSON.stringify(taskMap)); container.innerHTML=''; + let needsCanonicalSave=false; for(const task of _auxTasks){ const cfg=taskMap[task.task]||{provider:'auto',model:''}; const row=document.createElement('div'); @@ -12619,7 +12634,8 @@ async function _loadAuxiliaryModels(){ const modelSel=document.createElement('select'); modelSel.id='aux-model-'+task.task; modelSel.style.cssText=_auxSelectStyle(); - _buildAuxModelOptions(modelSel,cfg.provider,_auxProviders,cfg.model); + const canonicalModel=_buildAuxModelOptions(modelSel,cfg.provider,_auxProviders,cfg.model); + if(canonicalModel!==cfg.model) needsCanonicalSave=true; modelSel.addEventListener('change',()=>_onAuxModelChange(task.task)); row.appendChild(modelSel); @@ -12636,9 +12652,9 @@ async function _loadAuxiliaryModels(){ container.appendChild(row); } - // Hide apply button (no changes yet) + // Matching legacy @provider:model values can be repaired with one explicit Apply. const applyBtn=$('btnApplyAuxModels'); - if(applyBtn) applyBtn.style.display='none'; + if(applyBtn) applyBtn.style.display=needsCanonicalSave?'':'none'; // Reset button const resetBtn=$('btnResetAuxModels'); diff --git a/tests/test_auxiliary_model_provider_prefix.py b/tests/test_auxiliary_model_provider_prefix.py new file mode 100644 index 00000000..c7dabf16 --- /dev/null +++ b/tests/test_auxiliary_model_provider_prefix.py @@ -0,0 +1,253 @@ +"""Regression coverage for provider-qualified auxiliary model persistence. + +``GET /api/models`` may expose WebUI-only ``@provider:model`` routing IDs, but +auxiliary configuration stores provider and model in separate fields. The +provider-native model value must be used by both the settings UI and the shared +backend persistence boundary. +""" + +from __future__ import annotations + +import json +import shutil +import subprocess +from pathlib import Path + +import pytest + + +REPO = Path(__file__).resolve().parents[1] +PANELS_JS_PATH = REPO / "static" / "panels.js" +NODE = shutil.which("node") + + +@pytest.mark.skipif(NODE is None, reason="node not available") +def test_auxiliary_picker_uses_provider_native_model_values(): + """A custom-provider catalog must render and select provider-native values.""" + script = r""" +const fs = require('fs'); +const src = fs.readFileSync(process.argv[1], 'utf8'); + +function extract(name){ + const re = new RegExp('function\\s+' + name + '\\s*\\('); + const start = src.search(re); + if(start < 0) return ''; + let i = src.indexOf('{', start); + let depth = 0; + while(i < src.length){ + const ch = src[i]; + if(ch === '{') depth += 1; + else if(ch === '}'){ + depth -= 1; + if(depth === 0) return src.slice(start, i + 1); + } + i += 1; + } + throw new Error(name + ' parse failed'); +} + +global.t = () => ''; +global.document = { + createElement: () => ({value:'', textContent:'', selected:false}), +}; + +function makeSelect(){ + return { + children: [], + _value: '', + set innerHTML(_value){ this.children = []; this._value = ''; }, + get innerHTML(){ return ''; }, + get options(){ return this.children; }, + appendChild(opt){ this.children.push(opt); }, + insertBefore(opt, before){ + const idx = this.children.indexOf(before); + if(idx < 0) this.children.push(opt); + else this.children.splice(idx, 0, opt); + }, + set value(value){ + this._value = value; + for(const opt of this.children) opt.selected = opt.value === value; + }, + get value(){ + const selected = this.children.find((opt) => opt.selected); + return selected ? selected.value : this._value; + }, + }; +} + +const sharedHelper = extract('_modelBareNameForProvider'); +const buildOptions = extract('_buildAuxModelOptions'); +if(!buildOptions) throw new Error('_buildAuxModelOptions not found'); +eval(sharedHelper + '\n' + buildOptions); + +const select = makeSelect(); +const providers = [{ + slug: 'my-local-ai-gateway', + name: 'My Local AI Gateway', + models: [ + { + id: '@my-local-ai-gateway:example-chat-model', + label: 'Example Chat Model', + }, + { + id: '@my-local-ai-gateway:example-side-model', + label: '@my-local-ai-gateway:example-side-model', + }, + {id: 'vendor/example-model', label: 'Vendor Example Model'}, + {id: 'example-chat-model', label: 'Duplicate Chat Model'}, + ], +}]; + +const canonicalCurrent = _buildAuxModelOptions( + select, + 'my-local-ai-gateway', + providers, + '@my-local-ai-gateway:example-side-model', +); + +const modelOptions = select.options.filter( + (opt) => opt.value && opt.value !== '__custom__', +); +const helperCases = sharedHelper ? [ + _modelBareNameForProvider('@custom:router-alias:chat-model', 'custom'), + _modelBareNameForProvider('@custom:backup:model:free', 'custom:backup'), + _modelBareNameForProvider('vendor/example-model', 'my-local-ai-gateway'), + _modelBareNameForProvider('@other-gateway:other-model', 'my-local-ai-gateway'), +] : null; + +console.log(JSON.stringify({ + values: modelOptions.map((opt) => opt.value), + labels: modelOptions.map((opt) => opt.textContent), + selected: select.value, + canonicalCurrent, + helperCases, +})); +""" + + proc = subprocess.run( + [NODE, "-e", script, str(PANELS_JS_PATH)], + capture_output=True, + text=True, + timeout=20, + ) + assert proc.returncode == 0, f"node probe failed:\n{proc.stderr}" + result = json.loads(proc.stdout.strip().splitlines()[-1]) + + assert result == { + "values": [ + "example-chat-model", + "example-side-model", + "vendor/example-model", + ], + "labels": [ + "Example Chat Model", + "example-side-model", + "Vendor Example Model", + ], + "selected": "example-side-model", + "canonicalCurrent": "example-side-model", + "helperCases": [ + "router-alias:chat-model", + "model:free", + "vendor/example-model", + "@other-gateway:other-model", + ], + } + + +def test_matching_legacy_value_exposes_explicit_apply_repair(): + """Loading a safe legacy prefix should offer, but not force, a config write.""" + source = PANELS_JS_PATH.read_text(encoding="utf-8") + + assert "if(canonicalModel!==cfg.model) needsCanonicalSave=true;" in source + assert "applyBtn.style.display=needsCanonicalSave?'':'none'" in source + + +@pytest.mark.parametrize( + ("provider", "requested_model", "persisted_model"), + [ + ( + "my-local-ai-gateway", + "@my-local-ai-gateway:example-side-model", + "example-side-model", + ), + ( + "custom", + "@custom:router-alias:chat-model", + "router-alias:chat-model", + ), + ("custom:backup", "@custom:backup:model:free", "model:free"), + ( + "my-local-ai-gateway", + "vendor/example-model", + "vendor/example-model", + ), + ( + "my-local-ai-gateway", + "example-side-model", + "example-side-model", + ), + ], +) +def test_set_auxiliary_model_persists_provider_native_model( + monkeypatch, + tmp_path, + provider, + requested_model, + persisted_model, +): + from api import config + + config_path = tmp_path / "config.yaml" + config_path.write_text( + "auxiliary:\n vision:\n provider: auto\n model: ''\n", + encoding="utf-8", + ) + monkeypatch.setattr(config, "_get_config_path", lambda: config_path) + monkeypatch.setattr(config, "reload_config", lambda: None) + monkeypatch.setattr( + config, + "resolve_model_provider", + lambda model: (model, provider, None), + ) + + result = config.set_auxiliary_model("vision", provider, requested_model) + + saved = config._load_yaml_config_file(config_path)["auxiliary"]["vision"] + assert saved["provider"] == provider + assert saved["model"] == persisted_model + assert result["provider"] == provider + assert result["model"] == persisted_model + + +@pytest.mark.parametrize( + ("provider", "model"), + [ + ("my-local-ai-gateway", "@other-gateway:other-model"), + ("my-local-ai-gateway", "@my-local-ai-gateway:"), + ("auto", "@my-local-ai-gateway:example-side-model"), + ], +) +def test_set_auxiliary_model_rejects_invalid_qualified_pair_without_write( + monkeypatch, + tmp_path, + provider, + model, +): + from api import config + + config_path = tmp_path / "config.yaml" + original = ( + "auxiliary:\n" + " vision:\n" + " provider: openai\n" + " model: gpt-5.5\n" + ) + config_path.write_text(original, encoding="utf-8") + monkeypatch.setattr(config, "_get_config_path", lambda: config_path) + monkeypatch.setattr(config, "reload_config", lambda: None) + + with pytest.raises(ValueError, match="provider-qualified auxiliary model"): + config.set_auxiliary_model("vision", provider, model) + + assert config_path.read_text(encoding="utf-8") == original diff --git a/tests/test_issue4813_cron_model_bare_name.py b/tests/test_issue4813_cron_model_bare_name.py index bc3cbc0a..506783dd 100644 --- a/tests/test_issue4813_cron_model_bare_name.py +++ b/tests/test_issue4813_cron_model_bare_name.py @@ -39,7 +39,7 @@ def _fn_body(name: str) -> str: def _strip(model, provider): - body = _fn_body("_cronModelBareName") + body = _fn_body("_modelBareNameForProvider") + "\n" + _fn_body("_cronModelBareName") script = ( body + "\nconst args = "