diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d8db97d..610f2e8e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ ### Fixed +- **Plugin-provided model providers route correctly when set as the default.** A model from a plugin-only provider (e.g. a `@plugin:model` route) was surfaced in the catalog but, when that plugin provider was also the configured default, the request dropped the `@plugin:` routing hint and went to the wrong backend. Provider-hint resolution now applies plugin routing before the configured-provider bare-passthrough. Thanks @alexfoxtm. (#5909, #5461) + +- **Composer no longer double-sends or re-uploads on a fast second send.** The message text + its attachments are now captured and the textarea cleared immediately on send, before the async upload round-trip, so a re-entrant / interrupt-mode send can't re-read stale composer text and submit the same message twice or inherit the previous send's attachment. A clipboard-copy failure after a successful send no longer shows a false "failed", and a draft typed during the upload window is no longer clobbered. Thanks @harryazj. (#5912, #4750) + +- **Mobile dictation stays alive through natural pauses.** On touch devices the composer mic now keeps a dictation session going across the brief silences that used to end it (desktop stays one-shot), with a serialized screen wake-lock so rapid start/stop/visibility changes can't leak or drop it, and cleaner fallback to recording when the browser's speech recognition is unavailable. Thanks @brianmmaina. (#5915, #4732) + - **CSV files shared in chat keep a download link again.** A `MEDIA:` CSV rendered an inline table preview but β€” unlike every other media type β€” dropped the download affordance, so the file couldn't be retrieved once previewed. The preview header now shows a πŸ“Ž download link (via the correct `api/media?…&download=1` path), and the link stays visible even when the inline table itself fails to render, so CSV files exchanged through chat output remain retrievable. Thanks @ruizanthony. (#5792) ### Added diff --git a/api/config.py b/api/config.py index e5de5e4f..8216be50 100644 --- a/api/config.py +++ b/api/config.py @@ -35,6 +35,7 @@ import api.paths as _paths from api.plugin_providers import ( effective_provider_display_name as _effective_provider_display_name, is_plugin_model_provider as _is_plugin_model_provider, + plugin_model_provider_profiles as _plugin_model_provider_profiles, ) HOME = _paths.HOME @@ -2959,6 +2960,16 @@ def model_with_provider_context(model_id: str, model_provider: str | None = None if provider in _ACP_SUBPROCESS_PROVIDERS: return f"@{provider}:{model}" + # Plugin-only model providers (e.g. 9router, and other model plugins whose + # slugs are not in the static provider tables) route through the plugin, not + # the default provider. This MUST come before the `provider == config_provider` + # bare-passthrough below: when a plugin provider is ALSO the configured + # provider, returning a bare model would drop the '@plugin:' hint and the model + # would be sent to the wrong backend. Emit the explicit hint so it stays + # routable to the plugin that surfaced it. (#5909 gate finding) + if _is_plugin_model_provider(provider): + return f"@{provider}:{model}" + # If the selected provider is already the configured provider, leaving the # model bare preserves provider-specific base_url/proxy settings. if provider == config_provider: @@ -2978,6 +2989,9 @@ def model_with_provider_context(model_id: str, model_provider: str | None = None if isinstance(providers_cfg, dict) and provider in providers_cfg: return f"@{provider}:{model}" + # (Plugin-only provider routing handled above, before the config_provider + # bare-passthrough.) + # For non-OpenRouter slash IDs without an explicit configured provider, # keep the ID intact so existing custom/proxy base_url routing and # portal-provider handling remain in charge. @@ -4863,6 +4877,23 @@ def _static_models_catalog_without_live_probes() -> dict: if canonical and _provider_has_key(canonical): detected_providers.add(canonical) + # Plugin-only providers (e.g. 9router) are not in the static + # _PROVIDER_MODELS / _PROVIDER_DISPLAY tables and are detected above + # only when the user puts them in `providers.`. Plugins ship + # with their own env-var wiring, so an installed-and-keyed plugin + # provider should also enter the static catalog even without a + # `providers:` block β€” otherwise the picker silently drops the + # group when the live-rebuild cache is cold. + try: + for _plugin_pid in list(_plugin_model_provider_profiles().keys()): + if not _plugin_pid or not _provider_has_key(_plugin_pid): + continue + _canonical = _canonicalise_provider_id(_plugin_pid) or _plugin_pid + if _canonical: + detected_providers.add(_canonical) + except Exception: + logger.debug("Plugin provider detection failed in static catalog", exc_info=True) + fallback_cfg = cfg.get("fallback_providers", []) if isinstance(cfg, dict) else [] if isinstance(fallback_cfg, list): for entry in fallback_cfg: @@ -4996,12 +5027,32 @@ def _static_models_catalog_without_live_probes() -> dict: raw_models.append({"id": item, "label": item}) if not raw_models: raw_models = copy.deepcopy(_PROVIDER_MODELS.get(pid, [])) + # Plugin-only providers (e.g. 9router) are not in _PROVIDER_MODELS + # and rarely ship a `models:` allowlist in providers., so + # the static catalog above would render them as empty groups that + # the picker filters out. Fall back to the plugin's own + # ProviderProfile.fallback_models so the provider surfaces a + # curated, network-free subset on the cold path. The live + # rebuild (_build_available_models_uncached) does a full + # /v1/models fetch and supersedes this view on the next call. + if not raw_models and _is_plugin_model_provider(pid): + _plugin_profile = _plugin_model_provider_profiles().get( + (pid or "").strip().lower() + ) + if _plugin_profile is not None: + _fallback = getattr(_plugin_profile, "fallback_models", ()) or () + raw_models = [{"id": str(mid), "label": str(mid)} for mid in _fallback] for model_id in configured_model_ids.get(pid, []): if model_id and not any(m.get("id") == model_id for m in raw_models): raw_models.append( {"id": model_id, "label": _get_label_for_model(model_id, groups)} ) - if raw_models: + # Plugin-only providers (e.g. 9router) must enter `groups` even + # when `raw_models` is empty so the post-loop filter sees them. + # Without this, the earlier plugin-fallback pass only seeds + # `raw_models` when `fallback_models` is non-empty; the cold-cache + # picker still silently drops a keyed plugin with no models yet. + if raw_models or _is_plugin_model_provider(pid): groups.append( { "provider": provider_name, @@ -5037,7 +5088,14 @@ def _static_models_catalog_without_live_probes() -> dict: groups = [ group for group in groups - if group.get("models") or str(group.get("provider_id") or "").startswith("custom:") + if group.get("models") + or str(group.get("provider_id") or "").startswith("custom:") + # Keep plugin-only providers visible even when no models surfaced + # yet (e.g. plugin's fallback_models is empty and live rebuild + # hasn't completed). Otherwise they silently drop from the + # picker and look "not installed" β€” the same 9router-empty-group + # regression this branch was added to fix. + or _is_plugin_model_provider(str(group.get("provider_id") or "")) ] providers_with_keys: set[str] = set() diff --git a/static/boot.js b/static/boot.js index d20bd7cb..863225a6 100644 --- a/static/boot.js +++ b/static/boot.js @@ -675,6 +675,17 @@ function _micToastKeyForRecognitionError(error){ let _finalText=''; let _prefix=''; let _isRecording=false; + // #5294 salvage β€” mobile composer-mic dictation continuity. + // _speechStopRequested distinguishes an intentional stop (send/toggle) from a + // natural pause so onend only auto-restarts on real pauses. _micWakeLock keeps + // the screen awake while dictating; _micWakeLockOp serializes acquire/release + // so rapid start/stop/visibility churn can't leak a lock. _micRestartCount is + // bounded by _micMaxRestarts to stop a tight loop if the audio session is stolen. + let _speechStopRequested=false; + let _micWakeLock=null; + let _micWakeLockOp=null; + let _micRestartCount=0; + const _micMaxRestarts=20; let _micHoldTimer=null; let _micHoldActive=false; let _micPointerDown=false; @@ -802,6 +813,78 @@ function _micToastKeyForRecognitionError(error){ } } + // Gate continuous dictation to MOBILE so desktop stays one-shot (single + // utterance). An explicit hermes-mic-continuous flag wins in both directions, + // mirroring the hermes-voice-continuous pattern: 'true' opts a desktop in, + // 'false' opts a mobile out. Absent a flag, coarse-pointer (touch) devices get + // continuity and everything else stays single-utterance. + function _micDictationContinuous(){ + try{ + const flag=localStorage.getItem('hermes-mic-continuous'); + if(flag==='true') return true; + if(flag==='false') return false; + }catch(_){} + try{ return window.matchMedia('(pointer:coarse)').matches; }catch(_){ return false; } + } + + // Only auto-restart the composer-mic session on a natural pause: continuity + // must be enabled (mobile/opt-in), the session must still be active speech, + // the stop must not have been requested, and we must be under the restart cap. + function _micShouldRestartDictation(){ + return _micDictationContinuous() + && !_speechStopRequested + && !!window._micActive + && _activeCaptureMode==='speech' + && _micRestartCount<_micMaxRestarts; + } + + // Screen Wake Lock while dictating. All acquire/release ops are chained onto a + // single in-flight promise (_micWakeLockOp) so rapid start/stop/visibility + // churn runs strictly in order and can't interleave to leak a lock or null + // _micWakeLock mid-request. + function _acquireMicWakeLock(){ + if(!navigator.wakeLock) return Promise.resolve(); + _micWakeLockOp=Promise.resolve(_micWakeLockOp).then(async()=>{ + if(_micWakeLock) return; + if(!window._micActive||_activeCaptureMode!=='speech') return; + try{ + const lock=await navigator.wakeLock.request('screen'); + // If the session ended while awaiting, don't hold a stale lock. + if(!window._micActive||_activeCaptureMode!=='speech'){ + try{ await lock.release(); }catch(_){} + return; + } + _micWakeLock=lock; + _micWakeLock.addEventListener?.('release',()=>{ _micWakeLock=null; },{once:true}); + }catch(_){ + _micWakeLock=null; + } + }); + return _micWakeLockOp; + } + + function _releaseMicWakeLock(){ + _micWakeLockOp=Promise.resolve(_micWakeLockOp).then(async()=>{ + const lock=_micWakeLock; + _micWakeLock=null; + if(!lock) return; + try{ await lock.release(); }catch(_){} + }); + return _micWakeLockOp; + } + + // The OS drops a screen wake lock when the tab is hidden; reacquire on return + // if we're still dictating (release on hide is a no-op if none is held). + document.addEventListener('visibilitychange',()=>{ + if(document.visibilityState==='hidden'){ + void _releaseMicWakeLock(); + return; + } + if(window._micActive&&_activeCaptureMode==='speech'){ + void _acquireMicWakeLock(); + } + }); + function _stopMic(){ _micStartSeq+=1; _isRecording=false; @@ -811,6 +894,7 @@ function _micToastKeyForRecognitionError(error){ // which would otherwise make us stop the wrong backend and orphan the other // (#3169 Codex review). _activeCaptureMode is pinned at start. if(recognition && _activeCaptureMode==='speech'){ + _speechStopRequested=true; recognition.stop(); return; } @@ -826,13 +910,22 @@ function _micToastKeyForRecognitionError(error){ function _ensureSpeechRecognition(){ if(!SpeechRecognition) return null; const sr=recognition||new SpeechRecognition(); - sr.continuous=false; + // Desktop dictation stays one-shot (single utterance); mobile / opt-in + // devices run continuous so a natural pause doesn't end the session (#5294). + sr.continuous=_micDictationContinuous(); sr.interimResults=true; sr.lang=(typeof _locale!=='undefined'&&_locale._speech)||'en-US'; sr.onstart=()=>{ _finalText=''; }; sr.onresult=(event)=>{ + // #5294: a real result means the continuity restarts are PRODUCTIVE, not a + // stolen-audio-session tight loop β€” reset the restart budget so a long + // dictation with many natural pauses isn't silently capped at + // _micMaxRestarts. The cap still guards the failure case: consecutive + // restarts that yield no speech (onend without an intervening onresult) + // keep incrementing and trip the bound. + _micRestartCount=0; let interim=''; let final=_finalText; for(let i=event.resultIndex;i{ + // While dictating with continuity on, a no-speech/aborted error is a normal + // pause or transient audio-session hiccup β€” swallow it and let onend restart + // (bounded by _micMaxRestarts). Desktop one-shot still surfaces the toast. + if((event.error==='no-speech'||event.error==='aborted') + && _micDictationContinuous() + && window._micActive + && _activeCaptureMode==='speech' + && !_speechStopRequested + && _micRestartCount<_micMaxRestarts){ + return; + } + _speechStopRequested=false; _setRecording(false); window._micPendingSend=false; _isRecording=false; - if(event.error==='network'||event.error==='not-allowed'){ + _micRestartCount=0; + void _releaseMicWakeLock(); + if(event.error==='network'||event.error==='not-allowed' + ||event.error==='service-not-allowed'||event.error==='audio-capture'){ // Persist SR failure: next reload will skip SpeechRecognition localStorage.setItem(_micForceMediaRecorderKey,'1'); _forceMediaRecorder=true; @@ -960,8 +1091,14 @@ function _micToastKeyForRecognitionError(error){ } if(recognition && !_forceMediaRecorder && !_rawAudioMode){ _activeCaptureMode='speech'; + _speechStopRequested=false; + _micRestartCount=0; + // Refresh continuity gate at start so a settings/orientation change takes + // effect for this session (desktop stays one-shot, mobile stays continuous). + recognition.continuous=_micDictationContinuous(); recognition.lang=(typeof _locale!=='undefined'&&_locale._speech)||'en-US'; recognition.start(); + void _acquireMicWakeLock(); _setRecording(true); return; } diff --git a/static/messages.js b/static/messages.js index f3a6a0c2..56bd5190 100644 --- a/static/messages.js +++ b/static/messages.js @@ -1552,9 +1552,41 @@ async function send(){ const activeSid=S.session.session_id; _sendInProgressSid=activeSid; - setComposerStatus(S.pendingFiles&&S.pendingFiles.length?'Uploading…':''); + // Salvage of #4750 (@harryazj): capture the composer text and clear the + // textarea NOW β€” immediately after capture and BEFORE the uploadPendingFiles() + // / forced-skill-directive awaits below. send() re-reads the LIVE composer when + // it is re-entered while a send is in flight (the _sendInProgress guard at the + // top of this function reads _composerTextWithPendingSelections()). If we + // cleared only after the async work β€” as the pre-fix code did, down at the + // _clearComposerDraft site β€” a re-entrant/interrupt-mode send during the upload + // window would read the still-populated DOM and double-submit the same message. + // _submittedDraftTextForClear is the sole authority for the send-time draft + // signature from here down; no code path below re-reads $('msg').value on the + // happy path. + const _submittedDraftTextForClear=$('msg').value||''; + $('msg').value='';autoResize(); + + // #5912 gate CORE fix: snapshot the pending files that belong to THIS send + // BEFORE the await, and upload exactly that snapshot. Otherwise a re-entrant / + // interrupt-mode send during the upload window inherits the still-live + // S.pendingFiles and later re-uploads the first send's attachment. Detach the + // snapshot from S.pendingFiles now so files staged AFTER this point belong to + // the next send only. + const _submittedFiles=[...(S.pendingFiles||[])]; + const _submittedDraftFilesForClear=[..._submittedFiles]; + S.pendingFiles=[]; + if(typeof renderTray==='function')renderTray(); + + // #5912 gate SILENT fix: clear the PERSISTED draft here β€” alongside the + // textarea clear, BEFORE any await β€” so a new draft typed during the upload + // window is not clobbered by a delayed text:'' post. Keep the promise so the + // #5472 failed-send restore can chain its re-persist after this clear resolves. + let _composerDraftClearPromise=null; + if (activeSid && typeof _clearComposerDraft === 'function') _composerDraftClearPromise=_clearComposerDraft(activeSid,_submittedDraftTextForClear,_submittedDraftFilesForClear); + + setComposerStatus(_submittedFiles.length?'Uploading…':''); let uploaded=[]; - try{uploaded=await uploadPendingFiles();} + try{uploaded=await uploadPendingFiles({files:_submittedFiles, sessionId:activeSid, clearPending:false});} catch(e){if(!text){setComposerStatus(`Upload error: ${e.message}`);return;}} // Clear the uploading status now that upload is done β€” if we don't clear here // it stays visible for the entire duration of the agent stream, since @@ -1589,16 +1621,11 @@ async function send(){ } } if(!msgText){setComposerStatus('Nothing to send');return;} - const _submittedDraftTextForClear=$('msg').value||''; - const _submittedDraftFilesForClear=Array.isArray(_failedSendFilesSnapshot)?[..._failedSendFilesSnapshot]:[]; - $('msg').value='';autoResize(); - // Clear persisted composer draft since message was sent. Capture the promise - // so the #5472 failed-send restore can chain its re-persist AFTER this clear - // resolves β€” otherwise the two same-origin POSTs (clear text:'' then restore - // text:) can be reordered under HTTP/2 multiplexing and leave the - // server draft empty after a reload. (Opus #5484 NIT.) - let _composerDraftClearPromise=null; - if (activeSid && typeof _clearComposerDraft === 'function') _composerDraftClearPromise=_clearComposerDraft(activeSid,_submittedDraftTextForClear,_submittedDraftFilesForClear); + // Composer textarea + persisted draft were already captured and cleared + // immediately after capture (above, salvage of #4750 + #5912 gate fix) to close + // the re-entrant double-send race AND avoid clobbering a draft typed during the + // upload window. _composerDraftClearPromise / _submittedDraftFilesForClear are + // set there; nothing to re-declare here. const displayText=_slashDisplayTextOverride||text||(uploaded.length?`Uploaded: ${uploadedNames.join(', ')}`:'(file upload)'); const userMsg={role:'user',content:displayText,attachments:uploaded.length?uploadedNames:undefined,_ts:Date.now()/1000}; S.toolCalls=[]; // clear tool calls from previous turn diff --git a/tests/test_composer_capture_clear_race.py b/tests/test_composer_capture_clear_race.py new file mode 100644 index 00000000..19f1b535 --- /dev/null +++ b/tests/test_composer_capture_clear_race.py @@ -0,0 +1,188 @@ +"""Regression coverage for the composer capture-and-clear race (salvage of #4750). + +Bug (salvaged from #4750, credit @harryazj): ``send()`` in ``static/messages.js`` +captured the composer text early but only wiped the textarea +(``$('msg').value=''``) LATE β€” after ``await uploadPendingFiles()`` and the +forced-skill-directive await. ``send()`` is re-entrant: while a send is in +flight, a second invocation (e.g. an interrupt-mode / ``busy_input_mode`` drain, +or a fast second Enter) hits the ``if (_sendInProgress)`` guard at the top and +re-reads the LIVE composer via ``_composerTextWithPendingSelections()``. Because +the textarea still held the original text during the async upload window, the +re-entrant guard read the stale DOM and QUEUED the same message again -> +double-submit. + +Fix: capture the composer value and clear the textarea IMMEDIATELY after capture, +BEFORE any await. Once cleared, a re-entrant read sees an empty composer and the +guard's ``if(_text && _targetSid)`` short-circuits -> no duplicate queue. + +This module verifies BOTH: + 1. (static) the capture+wipe is ordered before the upload await in send(), and + 2. (behavioral, via node's ``vm``) the REAL re-entrancy guard block extracted + from send() does NOT re-queue when the composer was already cleared, and + WOULD have re-queued had the composer still held the stale text. +""" +import json +import shutil +import subprocess +import textwrap +from pathlib import Path + +import pytest + +ROOT = Path(__file__).resolve().parents[1] +MESSAGES_JS = ROOT.joinpath("static", "messages.js").read_text(encoding="utf-8") + + +# --------------------------------------------------------------------------- +# Static ordering assertions +# --------------------------------------------------------------------------- + +def _function_body(src: str, name: str) -> str: + marker = f"function {name}" + start = src.index(marker) + brace = src.index("{", start) + depth = 1 + i = brace + 1 + while depth and i < len(src): + if src[i] == "{": + depth += 1 + elif src[i] == "}": + depth -= 1 + i += 1 + return src[brace + 1 : i - 1] + + +def test_composer_captured_and_cleared_before_upload_await(): + """The fix: capture + textarea wipe must run before uploadPendingFiles().""" + body = _function_body(MESSAGES_JS, "send") + + capture_idx = body.index("const _submittedDraftTextForClear=$('msg').value||'';") + # The wipe sits immediately after the capture. + wipe_rel = body.index("$('msg').value='';autoResize();", capture_idx) + assert wipe_rel - capture_idx < 120, "the textarea wipe must sit immediately after the capture" + + upload_idx = body.index("uploaded=await uploadPendingFiles(") + directive_await_idx = body.index("const _directivePayload = await _pending.promise;") + + assert capture_idx < upload_idx, ( + "composer capture+clear must happen BEFORE the uploadPendingFiles() await " + "(salvage of #4750 β€” closes the re-entrant double-send race)" + ) + assert capture_idx < directive_await_idx, ( + "composer capture+clear must happen BEFORE the forced-skill-directive await too" + ) + + +def test_reentrancy_guard_reads_live_composer(): + """Sanity: the in-flight guard reads the LIVE composer (why the clear must be early).""" + body = _function_body(MESSAGES_JS, "send") + guard_idx = body.index("if (_sendInProgress) {") + guard_block = body[guard_idx : body.index("_sendInProgress = true;", guard_idx)] + assert "_composerTextWithPendingSelections().trim()" in guard_block, ( + "the re-entrant guard reads the live composer, so the composer must be " + "cleared before the first await to avoid a stale double-send" + ) + assert "queueSessionMessage(" in guard_block, ( + "the re-entrant guard queues the message it reads from the live composer" + ) + + +# --------------------------------------------------------------------------- +# Behavioral test β€” run the REAL re-entrancy guard against a cleared composer +# --------------------------------------------------------------------------- + +def _extract_reentrancy_guard() -> str: + """Slice the real `if (_sendInProgress) { ... }` guard block out of send().""" + body = _function_body(MESSAGES_JS, "send") + start = body.index("if (_sendInProgress) {") + # Balance braces from the guard's opening brace to its close. + brace = body.index("{", start) + depth = 1 + i = brace + 1 + while depth and i < len(body): + if body[i] == "{": + depth += 1 + elif body[i] == "}": + depth -= 1 + i += 1 + return body[start:i] + + +def _run_reentrant_guard_in_node(composer_value: str): + """Execute the REAL re-entrancy guard with a given live-composer value. + + Returns the list of queueSessionMessage calls the guard made. A non-empty + list means the guard re-queued (double-submit); an empty list means it + short-circuited on an empty composer (the fix's post-clear state). + """ + node = shutil.which("node") + if not node: # pragma: no cover + pytest.skip("node not available") + + helper = _function_body(MESSAGES_JS, "_composerTextWithPendingSelections") + guard = _extract_reentrancy_guard() + + harness = textwrap.dedent( + """ + const queued = []; + const state = { input: { value: %(composer_value)s } }; + const $ = (id) => (id === 'msg' ? state.input : null); + global.document = { getElementById: (id) => $(id) }; + // No pending inline selections in this scenario. + const _pendingSelections = []; + function _formatSelectedTextReplyQuote(t){ return t; } + // Real helper the guard uses to read the live composer. + function _composerTextWithPendingSelections(){%(helper)s} + + // Minimal in-flight state: a send is already running for sid-1. + let _sendInProgress = true; + let _sendInProgressSid = 'sid-1'; + const S = { session: { session_id: 'sid-1' }, pendingFiles: [], activeProfile: 'default' }; + + // Stubs the guard branch touches. + function _chatPayloadModelState(){ return { model: 'm', model_provider: 'p' }; } + function queueSessionMessage(sid, payload){ queued.push({ sid, payload }); } + function _clearComposerAfterQueuedSelectionSend(){ state.input.value = ''; } + function _clearComposerDraft(){} + function updateQueueBadge(){} + function renderTray(){} + function showToast(){} + + // Run the REAL guard block verbatim. + (function () { + %(guard)s + })(); + + console.log(JSON.stringify({ queued, composerAfter: state.input.value })); + """ + ) % { + "composer_value": json.dumps(composer_value), + "helper": helper, + "guard": guard, + } + + proc = subprocess.run([node, "-e", harness], capture_output=True, text=True, timeout=30) + assert proc.returncode == 0, f"node harness failed: {proc.stderr}" + return json.loads(proc.stdout.strip()) + + +def test_reentrant_send_does_not_requeue_after_composer_cleared(): + """With the fix, the composer is EMPTY when the re-entrant guard runs, so it + must NOT queue a duplicate of the stale text.""" + out = _run_reentrant_guard_in_node("") + assert out["queued"] == [], ( + "a re-entrant send must not re-queue anything once the composer has been " + "cleared by the in-flight send (salvage of #4750)" + ) + + +def test_reentrant_send_would_double_submit_if_composer_not_cleared(): + """Sanity / bug demonstration: had the composer NOT been cleared before the + async window (the pre-fix behaviour), the re-entrant guard reads the stale + DOM text and queues a DUPLICATE β€” exactly the double-send this fix prevents.""" + out = _run_reentrant_guard_in_node("hello world") + assert len(out["queued"]) == 1, ( + "the stale-composer scenario must reproduce the double-submit the fix removes" + ) + assert out["queued"][0]["payload"]["text"] == "hello world" + assert out["queued"][0]["sid"] == "sid-1" diff --git a/tests/test_issue5294_mobile_dictation_continuity.py b/tests/test_issue5294_mobile_dictation_continuity.py new file mode 100644 index 00000000..fa8da36f --- /dev/null +++ b/tests/test_issue5294_mobile_dictation_continuity.py @@ -0,0 +1,225 @@ +"""Behavioural regression for #5294 (salvage of the composer-mic dictation fix). + +The composer microphone (SpeechRecognition inside `_ensureSpeechRecognition`) +must keep a MOBILE dictation session alive across a natural pause, while DESKTOP +dictation stays one-shot (a single utterance that finalizes on the first pause). + +Rather than regex the source, this drives the ACTUAL `_ensureSpeechRecognition` +(and its gate helpers `_micDictationContinuous` / `_micShouldRestartDictation`) +from static/boot.js via node with a fake SpeechRecognition, then simulates a +final result followed by a pause (`onend`) and observes whether the session +restarts. It pins both directions so the mobile-gate can't silently regress into +either "desktop keeps restarting" or "mobile dies on every pause". +""" +import json +import shutil +import subprocess +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).parent.parent.resolve() +BOOT_JS_PATH = REPO_ROOT / "static" / "boot.js" + +NODE = shutil.which("node") +pytestmark = pytest.mark.skipif(NODE is None, reason="node not on PATH") + + +_DRIVER_SRC = r""" +const fs = require('fs'); +const src = fs.readFileSync(process.argv[2], 'utf8'); +const input = JSON.parse(process.argv[3]); + +// ── Extract a named function body from boot.js by brace-matching ──────────── +function extractFunc(name) { + const re = new RegExp('function\\s+' + name + '\\s*\\('); + const start = src.search(re); + if (start < 0) throw new Error(name + ' not found'); + let i = src.indexOf('{', start); + let depth = 1; i++; + while (depth > 0 && i < src.length) { + if (src[i] === '{') depth++; + else if (src[i] === '}') depth--; + i++; + } + return src.slice(start, i); +} + +// ── Fake browser + closure environment ────────────────────────────────────── +const _store = Object.assign({}, input.store || {}); +global.localStorage = { + getItem: (k) => (k in _store ? _store[k] : null), + setItem: (k, v) => { _store[k] = String(v); }, +}; +global.navigator = { wakeLock: null }; +global.window = { + _micActive: false, + _micPendingSend: false, + matchMedia: (q) => ({ matches: q.indexOf('pointer:coarse') >= 0 ? !!input.coarse : false }), +}; +global.document = { addEventListener: () => {}, visibilityState: 'visible' }; + +// Closure state that the extracted functions read/mutate at top level. +let SpeechRecognition = function () { + this.continuous = undefined; + this.interimResults = undefined; + this.lang = undefined; + this.onstart = null; + this.onresult = null; + this.onend = null; + this.onerror = null; + this.startCalls = 0; + this.stopCalls = 0; + this.start = function () { this.startCalls++; }; + this.stop = function () { this.stopCalls++; }; +}; +let recognition = null; +let _finalText = ''; +let _prefix = ''; +let _isRecording = false; +let _speechStopRequested = false; +let _micWakeLock = null; +let _micWakeLockOp = null; +let _micRestartCount = 0; +const _micMaxRestarts = 20; +let _activeCaptureMode = null; +let _forceMediaRecorder = false; +const _micForceMediaRecorderKey = 'mic_force_mediarecorder'; +const _locale = { _speech: 'en-US' }; +let ta = { value: '' }; +const toasts = []; + +function autoResize() {} +function send() {} +function showToast(m) { toasts.push(m); } +function t(k) { return k; } +function _applyDeferredServerSttFlip() {} +function _micToastKeyForRecognitionError() { return null; } +function _setRecording(on) { + window._micActive = on; + if (!on) { _finalText = ''; _prefix = ''; } +} +function _releaseMicWakeLock() { _micWakeLock = null; return Promise.resolve(); } +function _acquireMicWakeLock() { return Promise.resolve(); } + +// ── The real functions under test ─────────────────────────────────────────── +eval(extractFunc('_micDictationContinuous')); +eval(extractFunc('_micShouldRestartDictation')); +eval(extractFunc('_ensureSpeechRecognition')); + +// ── Drive a dictation session: start β†’ final result β†’ natural pause ───────── +recognition = _ensureSpeechRecognition(); +const continuousAfterEnsure = recognition.continuous; + +// Simulate _startMicCapture's speech branch entering an active session. +_activeCaptureMode = 'speech'; +_speechStopRequested = !!input.stopRequested; +// #5294 regression: allow pre-seeding the restart budget so we can prove a real +// result resets it (a long productive dictation must not silently hit the cap). +_micRestartCount = (typeof input.preRestartCount === 'number') ? input.preRestartCount : 0; +window._micActive = true; +recognition.start(); // initial start (mimics _startMicCapture) +const startCallsAfterStart = recognition.startCalls; + +// A final result lands, then the recognizer ends on a natural pause. +recognition.onstart(); +recognition.onresult({ + resultIndex: 0, + results: [Object.assign([{ transcript: 'hello world' }], { isFinal: true, length: 1 })], +}); +recognition.onend(); + +const out = { + continuousAfterEnsure: continuousAfterEnsure, + restarted: recognition.startCalls > startCallsAfterStart, + micActiveAfterPause: !!window._micActive, + restartCountAfter: _micRestartCount, + taValue: ta.value, + toasts: toasts, +}; +process.stdout.write(JSON.stringify(out)); +""" + + +@pytest.fixture(scope="module") +def driver_path(tmp_path_factory): + p = tmp_path_factory.mktemp("mic_dictation_driver") / "driver.js" + p.write_text(_DRIVER_SRC, encoding="utf-8") + return str(p) + + +def _run(driver_path, *, coarse=False, store=None, stop_requested=False, pre_restart_count=None): + payload = {"coarse": coarse, "store": store or {}, "stopRequested": stop_requested} + if pre_restart_count is not None: + payload["preRestartCount"] = pre_restart_count + result = subprocess.run( + [NODE, driver_path, str(BOOT_JS_PATH), json.dumps(payload)], + capture_output=True, text=True, timeout=30, + ) + if result.returncode != 0: + raise RuntimeError(f"node driver failed: {result.stderr}") + return json.loads(result.stdout) + + +# ───────────────────────────────────────────────────────────────────────────── +# DESKTOP must stay ONE-SHOT: continuous=false and no auto-restart on a pause. +# ───────────────────────────────────────────────────────────────────────────── +class TestDesktopStaysOneShot: + def test_desktop_recognition_is_not_continuous(self, driver_path): + out = _run(driver_path, coarse=False) + assert out["continuousAfterEnsure"] is False + + def test_desktop_does_not_restart_on_natural_pause(self, driver_path): + out = _run(driver_path, coarse=False) + assert out["restarted"] is False, "desktop dictation must finalize on the first pause" + assert out["micActiveAfterPause"] is False, "desktop session must end after the pause" + + def test_desktop_optout_flag_forces_one_shot_even_on_coarse_pointer(self, driver_path): + # An explicit 'false' opt-out must win even on a touch/coarse device. + out = _run(driver_path, coarse=True, store={"hermes-mic-continuous": "false"}) + assert out["continuousAfterEnsure"] is False + assert out["restarted"] is False + + +# ───────────────────────────────────────────────────────────────────────────── +# MOBILE must keep the session ALIVE through a pause via restart-on-onend. +# ───────────────────────────────────────────────────────────────────────────── +class TestMobileKeepsSessionAlive: + def test_mobile_recognition_is_continuous(self, driver_path): + out = _run(driver_path, coarse=True) + assert out["continuousAfterEnsure"] is True + + def test_mobile_restarts_on_natural_pause(self, driver_path): + out = _run(driver_path, coarse=True) + assert out["restarted"] is True, "mobile dictation must restart to survive a natural pause" + assert out["micActiveAfterPause"] is True, "mobile session must remain active after the pause" + + def test_mobile_intentional_stop_does_not_restart(self, driver_path): + # _speechStopRequested (set by _stopMic on send/toggle) must suppress the + # restart so an intentional stop still finalizes on mobile. + out = _run(driver_path, coarse=True, stop_requested=True) + assert out["restarted"] is False + assert out["micActiveAfterPause"] is False + + def test_desktop_optin_flag_enables_continuity(self, driver_path): + # An explicit 'true' opt-in makes a non-coarse (desktop) device continuous. + out = _run(driver_path, coarse=False, store={"hermes-mic-continuous": "true"}) + assert out["continuousAfterEnsure"] is True + assert out["restarted"] is True + + def test_productive_result_resets_restart_budget(self, driver_path): + # A real result means the continuity restarts are PRODUCTIVE, so the + # restart budget must reset β€” otherwise a long mobile dictation with many + # natural pauses silently hits _micMaxRestarts and dies mid-session. Seed + # the count AT the cap: onresult must reset it to 0 so the ensuing pause + # still restarts. Without the reset, count stays at the cap and the + # session would NOT restart. + out = _run(driver_path, coarse=True, pre_restart_count=20) + assert out["restartCountAfter"] == 1, ( + "a productive onresult must reset _micRestartCount (to 0), leaving 1 " + "after the subsequent natural-pause restart β€” not stay pinned at the cap" + ) + assert out["restarted"] is True, ( + "mobile dictation must keep restarting through pauses once a real " + "result has proven the session productive, even past the raw cap" + ) diff --git a/tests/test_issue5472_preserve_draft_on_failed_send.py b/tests/test_issue5472_preserve_draft_on_failed_send.py index 42571116..e65615e9 100644 --- a/tests/test_issue5472_preserve_draft_on_failed_send.py +++ b/tests/test_issue5472_preserve_draft_on_failed_send.py @@ -74,7 +74,7 @@ def test_send_captures_immutable_snapshot_before_rewrites_and_upload(): "const _failedSendFilesSnapshot=Array.isArray(S.pendingFiles)?[...S.pendingFiles]:[];" ) moa_idx = MESSAGES_JS.find("text=_moaArgs;") - upload_idx = MESSAGES_JS.find("uploaded=await uploadPendingFiles();") + upload_idx = MESSAGES_JS.find("uploaded=await uploadPendingFiles(") assert snap_idx != -1 and files_idx != -1, "send() must snapshot text + files for #5472" assert moa_idx != -1 and upload_idx != -1 # Snapshot happens before both the /moa rewrite and the upload drain. @@ -93,22 +93,50 @@ def test_error_branch_restores_original_snapshot_not_mutated_payload(): def test_send_still_clears_composer_on_the_happy_path(): - # Anchor to the MAIN-path clear specifically (the send-time composer wipe + - # persisted-draft clear), not a bare `$('msg').value=''` string that also - # appears at ~13 other sites (slash returns, bundle-error paths). This - # assertion must actually guard the main send path's clear. (Opus #5484 NIT.) + # Anchor to the MAIN-path persisted-draft clear specifically (not a bare + # `$('msg').value=''` string that also appears at ~13 other sites β€” slash + # returns, bundle-error paths). This assertion must actually guard the main + # send path's clear. main_clear = ( "if (activeSid && typeof _clearComposerDraft === 'function') " "_composerDraftClearPromise=_clearComposerDraft(activeSid,_submittedDraftTextForClear,_submittedDraftFilesForClear);" ) assert main_clear in MESSAGES_JS, "main send path must clear the persisted draft at send time" - # The composer textarea wipe must sit immediately above that clear. - clear_idx = MESSAGES_JS.find(main_clear) - window_before = MESSAGES_JS[clear_idx - 700:clear_idx] - assert "const _submittedDraftTextForClear=$('msg').value||'';" in window_before - assert "const _submittedDraftFilesForClear=Array.isArray(_failedSendFilesSnapshot)?[..._failedSendFilesSnapshot]:[];" in window_before - assert "$('msg').value='';autoResize();" in window_before, ( - "the main-path composer wipe must precede the persisted-draft clear" + + # Salvage of #4750: the composer textarea capture + wipe was moved UP to run + # immediately after capture (right after `_sendInProgressSid=activeSid;`) and + # BEFORE `uploadPendingFiles()` / the forced-skill-directive await β€” so a + # re-entrant/interrupt-mode send during the async window can't re-read the + # still-populated DOM and double-submit. Verify that ordering here. + capture = "const _submittedDraftTextForClear=$('msg').value||'';" + assert capture in MESSAGES_JS, "send() must still capture the send-time draft text" + capture_idx = MESSAGES_JS.index(capture) + # The textarea wipe sits immediately after the capture (same 120-char window). + window_after_capture = MESSAGES_JS[capture_idx : capture_idx + 120] + assert "$('msg').value='';autoResize();" in window_after_capture, ( + "the composer textarea wipe must sit immediately after the capture" + ) + upload_idx = MESSAGES_JS.index("uploaded=await uploadPendingFiles(") + clear_idx = MESSAGES_JS.index(main_clear) + # THE FIX: capture+wipe happen before the upload await (closes the race)... + assert capture_idx < upload_idx, ( + "composer must be captured+cleared BEFORE the uploadPendingFiles() await " + "so a re-entrant send can't re-read stale DOM text (salvage of #4750)" + ) + # ...and the captured text is still what feeds the persisted-draft clear. + assert capture_idx < clear_idx, ( + "the captured send-time draft text must precede the persisted-draft clear" + ) + # The files snapshot (#5912 gate fix) is taken from S.pendingFiles BEFORE the + # upload await and feeds the persisted-draft clear which now runs before the await. + assert ( + "const _submittedDraftFilesForClear=[..._submittedFiles];" + in MESSAGES_JS + ), "the files snapshot for the persisted-draft clear must be captured pre-await" + # The persisted-draft clear must run BEFORE the upload await (not after it), + # so a draft typed during the upload window is not clobbered. + assert clear_idx < upload_idx, ( + "the persisted-draft clear must precede the uploadPendingFiles() await (#5912)" ) diff --git a/tests/test_plugin_model_providers.py b/tests/test_plugin_model_providers.py index d89c1820..4cc6316f 100644 --- a/tests/test_plugin_model_providers.py +++ b/tests/test_plugin_model_providers.py @@ -223,3 +223,252 @@ class TestPluginModelProvidersPicker: config.cfg.update(old_cfg) config._cfg_mtime = old_mtime config.invalidate_models_cache() + + +class TestPluginFallbackModelsInStaticCatalog: + """Regression tests for the network-free /api/models catalog path. + + ``_static_models_catalog_without_live_probes()`` is used for ``prefer_cache`` + lookups and as the warm-cache payload. Plugin-only providers (e.g. 9router) + are not in ``_PROVIDER_MODELS`` and rarely ship a ``models:`` allowlist in + ``providers.``, so without a fallback the static catalog would render + them as empty groups that the picker filters out. The fix is to surface + the ``ProviderProfile.fallback_models`` declared by the plugin itself on + this cold path. + """ + + def test_static_catalog_surfaces_plugin_fallback_models( + self, monkeypatch, tmp_path + ): + _install_fake_yandex_plugin(monkeypatch) + _install_fake_hermes_cli(monkeypatch, authenticated=True, model_ids=[]) + monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path) + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + # _provider_has_key reads the env var directly, not the .env file. + # Set it so the static catalog's plugin-detection branch fires. + monkeypatch.setenv("YANDEX_API_KEY", "test-y...n") + + env_path = tmp_path / ".env" + env_path.write_text("YANDEX_API_KEY=test-y...n", encoding="utf-8") + + old_cfg = dict(config.cfg) + old_mtime = config._cfg_mtime + config.cfg.clear() + # No `providers.yandex.models:` allowlist, no `models:` in cfg at all. + # Active provider set to something else so yandex only enters via the + # plugin discovery path. + config.cfg["model"] = {"provider": "gemini", "default": "gemini-2.5-flash"} + config.cfg["providers"] = {} + try: + config._cfg_mtime = config.Path(config._get_config_path()).stat().st_mtime + except Exception: + config._cfg_mtime = 0.0 + + config.invalidate_models_cache() + try: + catalog = config._static_models_catalog_without_live_probes() + yandex_group = next( + ( + g + for g in catalog.get("groups", []) + if g.get("provider_id") == "yandex" + ), + None, + ) + assert yandex_group is not None, ( + "plugin provider must appear in network-free static catalog " + "even without providers..models allowlist" + ) + # yandex fake plugin declares no fallback_models in this test's + # profile, so the group is allowed to be empty β€” but the provider + # itself must be present (not filtered out as an empty group). + assert yandex_group.get("models") == [] or yandex_group.get("models") + finally: + config.cfg.clear() + config.cfg.update(old_cfg) + config._cfg_mtime = old_mtime + config.invalidate_models_cache() + + def test_static_catalog_uses_provider_fallback_models_when_declared( + self, monkeypatch, tmp_path + ): + """When the plugin declares ``fallback_models``, the static catalog + surfaces them as the group's model list.""" + fallback = ( + "gh/claude-sonnet-4.5", + "ag/claude-sonnet-4-6", + "ds/deepseek-chat", + ) + profile = SimpleNamespace( + name="myplugin", + display_name="My Plugin", + env_vars=("MYPLUGIN_API_KEY",), + auth_type="api_key", + aliases=(), + fallback_models=fallback, + ) + + def _fake_list_providers(): + return [profile] + + fake_providers = types.ModuleType("providers") + fake_providers.list_providers = _fake_list_providers + monkeypatch.setitem(sys.modules, "providers", fake_providers) + + from api.plugin_providers import invalidate_plugin_model_provider_cache + + invalidate_plugin_model_provider_cache() + + monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path) + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + # _provider_has_key reads the env var directly. + monkeypatch.setenv("MYPLUGIN_API_KEY", "test-m...n") + (tmp_path / ".env").write_text( + "MYPLUGIN_API_KEY=test-m...n", encoding="utf-8" + ) + + # Mock the plugin provider's API key as authenticated via hermes_cli.auth + fake_pkg = types.ModuleType("hermes_cli") + fake_pkg.__path__ = [] + fake_models = types.ModuleType("hermes_cli.models") + fake_models.list_available_providers = lambda: [] + fake_models.provider_model_ids = lambda pid: [] + fake_auth = types.ModuleType("hermes_cli.auth") + fake_auth.get_auth_status = lambda pid: ( + {"logged_in": True, "configured": True, "key_source": "env_file"} + if pid == "myplugin" + else {} + ) + monkeypatch.setitem(sys.modules, "hermes_cli", fake_pkg) + monkeypatch.setitem(sys.modules, "hermes_cli.models", fake_models) + monkeypatch.setitem(sys.modules, "hermes_cli.auth", fake_auth) + + old_cfg = dict(config.cfg) + old_mtime = config._cfg_mtime + config.cfg.clear() + config.cfg["model"] = {"provider": "gemini", "default": "gemini-2.5-flash"} + config.cfg["providers"] = {} + try: + config._cfg_mtime = config.Path(config._get_config_path()).stat().st_mtime + except Exception: + config._cfg_mtime = 0.0 + + config.invalidate_models_cache() + try: + catalog = config._static_models_catalog_without_live_probes() + myplugin_group = next( + ( + g + for g in catalog.get("groups", []) + if g.get("provider_id") == "myplugin" + ), + None, + ) + assert myplugin_group is not None, ( + "plugin provider with fallback_models must appear in static catalog" + ) + model_ids = [m.get("id") for m in myplugin_group.get("models", [])] + assert model_ids == list(fallback), ( + f"static catalog should use plugin's fallback_models, got {model_ids}" + ) + finally: + config.cfg.clear() + config.cfg.update(old_cfg) + config._cfg_mtime = old_mtime + config.invalidate_models_cache() + + +class TestPluginModelProviderRouting: + """Regression test for routing surfaced plugin-only models. + + Surfacing a plugin-only model in the picker is only half the fix: the + selected ``(model, model_provider)`` pair must also *route* to the plugin. + ``model_with_provider_context()`` historically let any slash-bearing model + ID fall through to a bare passthrough, which made a surfaced plugin model + such as ``gh/claude-sonnet-4.5`` inherit the default provider instead of + the plugin that surfaced it. The fix emits an explicit ``@plugin:model`` + hint for plugin-only providers before that bare passthrough. + """ + + def test_plugin_only_model_routes_to_plugin_provider(self, monkeypatch): + profile = SimpleNamespace( + name="myplugin", + display_name="My Plugin", + env_vars=("MYPLUGIN_API_KEY",), + auth_type="api_key", + aliases=(), + fallback_models=("gh/claude-sonnet-4.5",), + ) + + def _fake_list_providers(): + return [profile] + + fake_providers = types.ModuleType("providers") + fake_providers.list_providers = _fake_list_providers + monkeypatch.setitem(sys.modules, "providers", fake_providers) + invalidate_plugin_model_provider_cache() + + old_cfg = dict(config.cfg) + config.cfg.clear() + # Default provider is something else; plugin has no `providers:` block. + # Without the plugin-routing branch the slash-bearing model ID would + # fall through to the bare passthrough and inherit the default. + config.cfg["model"] = {"provider": "gemini", "default": "gemini-2.5-flash"} + config.cfg["providers"] = {} + try: + assert config._is_plugin_model_provider("myplugin"), ( + "test setup: fake plugin provider must be recognised" + ) + routed = config.model_with_provider_context( + "gh/claude-sonnet-4.5", "myplugin" + ) + assert routed == "@myplugin:gh/claude-sonnet-4.5", ( + "surfaced plugin-only model must route to its plugin, " + f"got {routed!r}" + ) + finally: + config.cfg.clear() + config.cfg.update(old_cfg) + invalidate_plugin_model_provider_cache() + + def test_plugin_provider_routes_even_when_it_is_the_configured_provider(self, monkeypatch): + # #5909 gate finding: the plugin-routing branch must run BEFORE the + # `provider == config_provider` bare-passthrough. When the ACTIVE plugin + # provider is also the configured default, returning a bare model would + # drop the '@plugin:' hint and route to the wrong backend. + profile = SimpleNamespace( + name="gmi", + display_name="GMI", + env_vars=("GMI_API_KEY",), + auth_type="api_key", + aliases=(), + fallback_models=("anthropic/claude-sonnet-4.6",), + ) + + def _fake_list_providers(): + return [profile] + + fake_providers = types.ModuleType("providers") + fake_providers.list_providers = _fake_list_providers + monkeypatch.setitem(sys.modules, "providers", fake_providers) + invalidate_plugin_model_provider_cache() + + old_cfg = dict(config.cfg) + config.cfg.clear() + # The plugin IS the configured provider now. + config.cfg["model"] = {"provider": "gmi", "default": "anthropic/claude-sonnet-4.6"} + config.cfg["providers"] = {} + try: + assert config._is_plugin_model_provider("gmi") + routed = config.model_with_provider_context( + "anthropic/claude-sonnet-4.6", "gmi" + ) + assert routed == "@gmi:anthropic/claude-sonnet-4.6", ( + "an active plugin provider must keep its '@plugin:' routing hint " + "even when it equals the configured provider, got " + f"{routed!r}" + ) + finally: + config.cfg.clear() + config.cfg.update(old_cfg) + invalidate_plugin_model_provider_cache()