Merge pull request #5923 from nesquena/release/stage-batch-reliability
Release exp-v0.52.34: salvage-reliability batch (#5909, #5912, #5915)
This commit is contained in:
8 files changed
+947
-29
No files matched your search
@@ -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
|
||||
|
||||
+60
-2
@@ -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.<slug>`. 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.<slug>, 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()
|
||||
|
||||
+140
-3
@@ -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<event.results.length;i++){
|
||||
@@ -850,9 +943,32 @@ function _micToastKeyForRecognitionError(error){
|
||||
? _prefix+' '+_finalText.trimStart()
|
||||
: _prefix+_finalText)
|
||||
: ta.value;
|
||||
_setRecording(false);
|
||||
ta.value=committed;
|
||||
autoResize();
|
||||
// Mobile / opt-in continuity: a natural pause ends this recognition run but
|
||||
// the user is still dictating, so restart to keep the session alive. Desktop
|
||||
// (one-shot) and intentional stops (_speechStopRequested) skip this and
|
||||
// finalize. Bounded by _micMaxRestarts so a stolen audio session can't loop.
|
||||
if(_micShouldRestartDictation()){
|
||||
_prefix=committed&&!committed.endsWith(' ')&&!committed.endsWith('\n')
|
||||
? committed+' '
|
||||
: committed;
|
||||
_finalText='';
|
||||
_micRestartCount++;
|
||||
try{
|
||||
sr.start();
|
||||
return;
|
||||
}catch(err){
|
||||
// Restart failed (e.g. the audio session was taken by another app).
|
||||
// Surface it instead of silently dropping to idle (Greptile P2).
|
||||
showToast(t('mic_error')+String((err&&err.message)||'restart'));
|
||||
}
|
||||
}
|
||||
_speechStopRequested=false;
|
||||
_isRecording=false;
|
||||
_micRestartCount=0;
|
||||
void _releaseMicWakeLock();
|
||||
_setRecording(false);
|
||||
if(window._micPendingSend){
|
||||
window._micPendingSend=false;
|
||||
send();
|
||||
@@ -861,10 +977,25 @@ function _micToastKeyForRecognitionError(error){
|
||||
};
|
||||
|
||||
sr.onerror=(event)=>{
|
||||
// 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;
|
||||
}
|
||||
|
||||
+39
-12
@@ -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:<draft>) 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
|
||||
|
||||
@@ -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"
|
||||
@@ -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"
|
||||
)
|
||||
@@ -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)"
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -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.<slug>``, 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.<slug>.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()
|
||||
Reference in new issue
Block a user