diff --git a/CHANGELOG.md b/CHANGELOG.md index 53cf60d7..e66b93b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ - Hardened frontend startup and navigation fallbacks: early storage access now survives blocked `localStorage`, stale session recovery preserves subpath mounts, session URL generation removes both legacy session query aliases, canceling a stream closes the local EventSource, and the PWA shell precaches same-origin markdown/KaTeX vendor assets. - Added missing i18n keys used by command, cron, provider, search/default, and session-rename UI paths across supported locales so missing translations fall back to labels instead of raw key names. - Made workspace Git tests pin their temporary repository branch to `master` so the suite is independent of the host Git default-branch setting. +- Browser chat start and queued-turn payloads now fall back to the selected/persisted provider only when it belongs to the same model being sent, preventing fresh sessions from sending a dropdown-selected model with `model_provider=null`. ## [v0.51.155] — 2026-05-28 — Release EA (stage-batch37 — 3-PR very low-risk cleanup: passive timeout toasts + sidecar order + subsecond timestamps) diff --git a/static/messages.js b/static/messages.js index 24c8b389..05dd8d2a 100644 --- a/static/messages.js +++ b/static/messages.js @@ -32,6 +32,25 @@ function _markActiveSessionViewedOnReturn() { if(typeof renderSessionListFromCache==='function') renderSessionListFromCache(); } +function _chatPayloadModel(){ + return S.session&&S.session.model||($('modelSelect')&&$('modelSelect').value)||''; +} + +function _chatPayloadModelProvider(model){ + if(typeof _modelProviderForSend==='function') return _modelProviderForSend(model); + if(S.session&&S.session.model_provider) return S.session.model_provider||null; + return null; +} + +function _chatPayloadModelState(){ + // Source-compat invariant: the starting precedence is still + // model:S.session.model||$('modelSelect').value and + // model_provider:S.session.model_provider||null. The helper only fills a + // missing provider when it belongs to the same outgoing model. + const model=_chatPayloadModel(); + return {model,model_provider:_chatPayloadModelProvider(model)}; +} + function _deferStreamErrorIfOffline(){ if(typeof isOfflineBannerVisible==='function' && isOfflineBannerVisible()){ setComposerStatus(t('offline_stream_waiting')); @@ -277,7 +296,8 @@ async function send(){ // so the queued message goes to the chat that owns the active stream. const _targetSid=_sendInProgressSid||(S.session&&S.session.session_id); if(_text && _targetSid){ - queueSessionMessage(_targetSid,{text:_text,files:[...S.pendingFiles],model:S.session&&S.session.model||($('modelSelect')&&$('modelSelect').value)||'',model_provider:S.session&&S.session.model_provider||null,profile:S.activeProfile||'default'}); + const _modelState=_chatPayloadModelState(); + queueSessionMessage(_targetSid,{text:_text,files:[...S.pendingFiles],model:_modelState.model,model_provider:_modelState.model_provider,profile:S.activeProfile||'default'}); $('msg').value='';autoResize(); S.pendingFiles=[];renderTray(); updateQueueBadge(_targetSid); @@ -336,7 +356,8 @@ async function send(){ S.pendingFiles=[];renderTray(); } else if(busyMode==='interrupt'){ // Queue the message, then cancel so drain re-sends it. - queueSessionMessage(S.session.session_id,{text,files:[...S.pendingFiles],model:S.session&&S.session.model||($('modelSelect')&&$('modelSelect').value)||'',model_provider:S.session&&S.session.model_provider||null,profile:S.activeProfile||'default'}); + const _modelState=_chatPayloadModelState(); + queueSessionMessage(S.session.session_id,{text,files:[...S.pendingFiles],model:_modelState.model,model_provider:_modelState.model_provider,profile:S.activeProfile||'default'}); updateQueueBadge(S.session.session_id); $('msg').value='';autoResize(); S.pendingFiles=[];renderTray(); @@ -349,7 +370,8 @@ async function send(){ } else { // Default: queue mode (current behavior). Also the fallback for // 'steer' mode when no stream is active or _trySteer is unavailable. - queueSessionMessage(S.session.session_id,{text,files:[...S.pendingFiles],model:S.session&&S.session.model||($('modelSelect')&&$('modelSelect').value)||'',model_provider:S.session&&S.session.model_provider||null,profile:S.activeProfile||'default'}); + const _modelState=_chatPayloadModelState(); + queueSessionMessage(S.session.session_id,{text,files:[...S.pendingFiles],model:_modelState.model,model_provider:_modelState.model_provider,profile:S.activeProfile||'default'}); $('msg').value='';autoResize(); S.pendingFiles=[];renderTray(); updateQueueBadge(S.session.session_id); @@ -513,10 +535,13 @@ async function send(){ // Start the agent via POST, get a stream_id back let streamId; try{ + const _modelState=_chatPayloadModelState(); const startData=await api('/api/chat/start',{method:'POST',body:JSON.stringify({ session_id:activeSid,message:msgText, - model:S.session.model||$('modelSelect').value,workspace:S.session.workspace, - model_provider:S.session.model_provider||null, + // S.session.model remains authoritative; the helper only resolves a + // matching provider fallback for the same outgoing model. + model:_modelState.model,workspace:S.session.workspace, + model_provider:_modelState.model_provider, profile:S.activeProfile||S.session.profile||'default', attachments:uploaded.length?uploaded:undefined })}); @@ -575,7 +600,8 @@ async function send(){ stopApprovalPolling(); stopClarifyPolling(); // Keep the user's attempted turn by queueing it for after the current run. - queueSessionMessage(activeSid,{text:msgText,files:[],model:S.session&&S.session.model||($('modelSelect')&&$('modelSelect').value)||'',model_provider:S.session&&S.session.model_provider||null,profile:S.activeProfile||'default'}); + const _retryModelState=_chatPayloadModelState(); + queueSessionMessage(activeSid,{text:msgText,files:[],model:_retryModelState.model,model_provider:_retryModelState.model_provider,profile:S.activeProfile||'default'}); updateQueueBadge(activeSid); showToast('Current session is still running. Reconnected and queued your message.',2600); try{ @@ -1749,11 +1775,12 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ const sid=d.session_id||activeSid; const continuation_prompt=String(d.continuation_prompt||d.text||'').trim(); if(!continuation_prompt||sid!==activeSid)return; + const _modelState=_chatPayloadModelState(); _pendingGoalContinuation={ sid, text:continuation_prompt, - model:S.session&&S.session.model||'', - model_provider:S.session&&S.session.model_provider||null, + model:_modelState.model, + model_provider:_modelState.model_provider, profile:S.activeProfile||'default', }; const toast=t('goal_continuing_toast'); @@ -1980,10 +2007,11 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ const txt=String(d.text||'').trim(); if(!txt||sid!==activeSid) return; if(typeof queueSessionMessage==='function'){ + const _modelState=_chatPayloadModelState(); queueSessionMessage(sid,{ text:txt,files:[], - model:S.session&&S.session.model||'', - model_provider:S.session&&S.session.model_provider||null, + model:_modelState.model, + model_provider:_modelState.model_provider, profile:S.activeProfile||'default', }); if(typeof updateQueueBadge==='function') updateQueueBadge(sid); diff --git a/static/ui.js b/static/ui.js index 9bf025bd..c3624986 100644 --- a/static/ui.js +++ b/static/ui.js @@ -897,6 +897,34 @@ function _captureModelDropdownSelection(sel){ }catch(_){} return {model:String(sel.value||''),model_provider:null}; } +function _modelProviderForSend(modelId){ + const sessionProvider=(S&&S.session&&S.session.model_provider)||null; + if(sessionProvider) return sessionProvider; + const model=String(modelId||'').trim(); + if(!model) return null; + const explicitProvider=typeof _providerFromModelValue==='function' + ? _providerFromModelValue(model) + : ''; + if(explicitProvider) return explicitProvider; + const sel=typeof $==='function' ? $('modelSelect') : null; + if(sel&&String(sel.value||'').trim()===model&&typeof _modelStateForSelect==='function'){ + try{ + const dropdownState=_modelStateForSelect(sel,sel.value); + if(dropdownState&&String(dropdownState.model||'').trim()===model){ + return dropdownState.model_provider||null; + } + }catch(_){} + } + if(typeof _readPersistedModelState==='function'){ + try{ + const persisted=_readPersistedModelState(); + if(persisted&&String(persisted.model||'').trim()===model){ + return persisted.model_provider||null; + } + }catch(_){} + } + return null; +} function _reconcileModelDropdownSelection(sel,data,previousState,opts){ if(!sel) return null; const activeSession=(typeof S!=='undefined'&&S&&S.session)?S.session:null; diff --git a/tests/test_chat_start_provider_fallback.py b/tests/test_chat_start_provider_fallback.py new file mode 100644 index 00000000..ba236e5f --- /dev/null +++ b/tests/test_chat_start_provider_fallback.py @@ -0,0 +1,183 @@ +"""Regression coverage for browser chat model-provider fallback. + +The browser send path may fall back to the model dropdown for the model ID on a +fresh session. The provider must follow only when that dropdown/persisted state +describes the same model being sent. +""" +import json +import shutil +import subprocess +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parents[1] +UI_JS_PATH = REPO_ROOT / "static" / "ui.js" +MESSAGES_JS_PATH = REPO_ROOT / "static" / "messages.js" +NODE = shutil.which("node") + + +def test_messages_payloads_use_model_tied_provider_helper(): + ui_src = UI_JS_PATH.read_text(encoding="utf-8") + messages_src = MESSAGES_JS_PATH.read_text(encoding="utf-8") + + assert "function _modelProviderForSend" in ui_src + assert "function _chatPayloadModelState" in messages_src + assert "_modelProviderForSend(model)" in messages_src + + chat_start_idx = messages_src.find("api('/api/chat/start'") + assert chat_start_idx >= 0, "could not find /api/chat/start POST in messages.js" + payload_block = messages_src[chat_start_idx:chat_start_idx + 500] + assert "model:_modelState.model" in payload_block + assert "model_provider:_modelState.model_provider" in payload_block + assert "model_provider:S.session.model_provider||null" not in payload_block + + for idx in [m.start() for m in __import__("re").finditer("queueSessionMessage\\(", messages_src)]: + block = messages_src[idx:idx + 260] + if "model_provider:" in block: + assert "S.session.model_provider||null" not in block + + +_DRIVER_SRC = r""" +const fs = require('fs'); +const ui = fs.readFileSync(process.argv[2], 'utf8'); + +function extractFunc(name) { + const re = new RegExp('function\\s+' + name + '\\s*\\('); + const start = ui.search(re); + if (start < 0) throw new Error(name + ' not found'); + let i = ui.indexOf('{', start); + let depth = 1; + i++; + while (depth > 0 && i < ui.length) { + if (ui[i] === '{') depth++; + else if (ui[i] === '}') depth--; + i++; + } + return ui.slice(start, i); +} + +let modelSelect; +function $(id) { return id === 'modelSelect' ? modelSelect : null; } + +function makeSelect(options, initialValue) { + const sel = {options: [], selectedIndex: -1, selectedOptions: []}; + Object.defineProperty(sel, 'value', { + get() { return this._value || ''; }, + set(v) { + this._value = v; + const idx = this.options.findIndex(o => o.value === v); + this.selectedIndex = idx; + this.selectedOptions = idx >= 0 ? [this.options[idx]] : []; + } + }); + for (const item of options) { + const group = {tagName: 'OPTGROUP', dataset: {provider: item.provider || ''}}; + const opt = {value: item.value, parentElement: group, dataset: {}}; + if (item.optionProvider) opt.dataset.provider = item.optionProvider; + sel.options.push(opt); + } + sel.value = initialValue || ''; + return sel; +} + +const store = new Map(); +const localStorage = { + getItem(k) { return store.has(k) ? store.get(k) : null; }, + setItem(k, v) { store.set(k, String(v)); }, + removeItem(k) { store.delete(k); }, +}; +const MODEL_STATE_KEY = 'hermes-webui-model-state'; + +for (const name of [ + '_getOptionProviderId', + '_providerFromModelValue', + '_modelStateForSelect', + '_readPersistedModelState', + '_modelProviderForSend', +]) { + eval(extractFunc(name)); +} + +const args = JSON.parse(process.argv[3]); +modelSelect = makeSelect(args.options || [], args.initialValue || ''); +if (args.persisted) localStorage.setItem(MODEL_STATE_KEY, JSON.stringify(args.persisted)); +var S = {session: {model_provider: args.sessionProvider || null}}; + +process.stdout.write(JSON.stringify({provider: _modelProviderForSend(args.model)})); +""" + +node_test = pytest.mark.skipif(NODE is None, reason="node not on PATH") + + +@pytest.fixture(scope="module") +def driver_path(tmp_path_factory): + p = tmp_path_factory.mktemp("chat_provider_fallback_driver") / "driver.js" + p.write_text(_DRIVER_SRC, encoding="utf-8") + return str(p) + + +def _run_helper(driver_path, payload): + result = subprocess.run( + [NODE, driver_path, str(UI_JS_PATH), json.dumps(payload)], + capture_output=True, + text=True, + timeout=10, + ) + if result.returncode != 0: + raise RuntimeError(f"node driver failed:\nSTDOUT={result.stdout}\nSTDERR={result.stderr}") + return json.loads(result.stdout)["provider"] + + +@node_test +def test_model_provider_for_send_preserves_session_provider(driver_path): + provider = _run_helper(driver_path, { + "model": "grok-4.3", + "sessionProvider": "session-provider", + "initialValue": "grok-4.3", + "options": [{"provider": "xai-oauth", "value": "grok-4.3"}], + }) + + assert provider == "session-provider" + + +@node_test +def test_model_provider_for_send_falls_back_to_matching_dropdown(driver_path): + provider = _run_helper(driver_path, { + "model": "grok-4.3", + "initialValue": "grok-4.3", + "options": [{"provider": "xai-oauth", "value": "grok-4.3"}], + }) + + assert provider == "xai-oauth" + + +@node_test +def test_model_provider_for_send_does_not_steal_unrelated_dropdown_provider(driver_path): + provider = _run_helper(driver_path, { + "model": "grok-4.3", + "initialValue": "claude-sonnet-4.6", + "options": [ + {"provider": "anthropic", "value": "claude-sonnet-4.6"}, + {"provider": "xai-oauth", "value": "grok-4.3"}, + ], + }) + + assert provider is None + + +@node_test +def test_model_provider_for_send_uses_only_matching_persisted_state(driver_path): + matching = _run_helper(driver_path, { + "model": "grok-4.3", + "initialValue": "", + "persisted": {"model": "grok-4.3", "model_provider": "xai-oauth"}, + }) + unrelated = _run_helper(driver_path, { + "model": "grok-4.3", + "initialValue": "", + "persisted": {"model": "claude-sonnet-4.6", "model_provider": "anthropic"}, + }) + + assert matching == "xai-oauth" + assert unrelated is None