mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
fix(stt): lock local model load; allow keyless local OpenAI-compatible STT
Two small fresh fixes on top of the salvage wave: - Wrap the check-then-load of the module-global faster-whisper model in a double-checked threading.Lock so concurrent voice messages can't both download/load the model (#24767). - Treat an empty stt.openai.api_key as no-auth when stt.openai.base_url points at a loopback/RFC-1918/.local host, so local OpenAI-compatible STT servers (faster-whisper-server, speaches, vLLM whisper) work without a sham api_key value. Reimplements the idea from PR #25193 — credit @nnnet. Co-authored-by: nnnet <nnnet@users.noreply.github.com>
This commit is contained in:
parent
ef0d8ce2c5
commit
9467dc135f
2 changed files with 151 additions and 13 deletions
|
|
@ -1988,3 +1988,102 @@ class TestShellSafety:
|
|||
monkeypatch.delenv(LOCAL_STT_COMMAND_ENV, raising=False)
|
||||
use_shell = bool(os.getenv(LOCAL_STT_COMMAND_ENV, "").strip())
|
||||
assert use_shell is False
|
||||
|
||||
|
||||
class TestLocalModelLock:
|
||||
"""#24767 — concurrent first-use must not double-load the whisper model."""
|
||||
|
||||
def test_lock_exists_and_is_a_lock(self):
|
||||
import threading
|
||||
from tools import transcription_tools
|
||||
assert isinstance(transcription_tools._local_model_lock, type(threading.Lock()))
|
||||
|
||||
def test_concurrent_transcribe_loads_model_once(self, tmp_path):
|
||||
import threading
|
||||
from tools import transcription_tools
|
||||
from tools.transcription_tools import _transcribe_local
|
||||
|
||||
audio = tmp_path / "test.ogg"
|
||||
audio.write_bytes(b"fake")
|
||||
|
||||
seg = MagicMock()
|
||||
seg.text = "hello"
|
||||
info = MagicMock()
|
||||
info.language = "en"
|
||||
info.duration = 1.0
|
||||
|
||||
load_count = 0
|
||||
load_started = threading.Event()
|
||||
|
||||
def slow_load(model_name, device="auto", compute_type="auto"):
|
||||
nonlocal load_count
|
||||
load_count += 1
|
||||
load_started.set()
|
||||
import time
|
||||
time.sleep(0.05)
|
||||
model = MagicMock()
|
||||
model.transcribe.return_value = ([seg], info)
|
||||
return model
|
||||
|
||||
with patch("tools.transcription_tools._HAS_FASTER_WHISPER", True), \
|
||||
patch("tools.transcription_tools._load_stt_config", return_value={}), \
|
||||
patch("tools.transcription_tools._load_local_whisper_model", side_effect=slow_load), \
|
||||
patch("tools.transcription_tools._local_model", None), \
|
||||
patch("tools.transcription_tools._local_model_name", None):
|
||||
threads = [
|
||||
threading.Thread(target=_transcribe_local, args=(str(audio), "base"))
|
||||
for _ in range(4)
|
||||
]
|
||||
for t in threads:
|
||||
t.start()
|
||||
for t in threads:
|
||||
t.join(timeout=10)
|
||||
|
||||
assert load_count == 1
|
||||
|
||||
|
||||
class TestLocalBaseUrlNoApiKey:
|
||||
"""#25193 — empty api_key with a local base_url should not raise."""
|
||||
|
||||
def test_local_base_url_returns_placeholder_key(self):
|
||||
from tools.transcription_tools import _resolve_openai_audio_client_config
|
||||
with patch(
|
||||
"tools.transcription_tools._load_stt_config",
|
||||
return_value={"openai": {"base_url": "http://localhost:8504/v1"}},
|
||||
):
|
||||
api_key, base_url = _resolve_openai_audio_client_config()
|
||||
assert api_key == "not-needed"
|
||||
assert base_url == "http://localhost:8504/v1"
|
||||
|
||||
def test_private_ip_base_url_returns_placeholder_key(self):
|
||||
from tools.transcription_tools import _resolve_openai_audio_client_config
|
||||
with patch(
|
||||
"tools.transcription_tools._load_stt_config",
|
||||
return_value={"openai": {"base_url": "http://192.168.1.10:8000/v1"}},
|
||||
):
|
||||
api_key, base_url = _resolve_openai_audio_client_config()
|
||||
assert api_key == "not-needed"
|
||||
|
||||
def test_public_base_url_still_requires_key(self):
|
||||
from tools.transcription_tools import _resolve_openai_audio_client_config
|
||||
with patch(
|
||||
"tools.transcription_tools._load_stt_config",
|
||||
return_value={"openai": {"base_url": "https://api.example.com/v1"}},
|
||||
), patch(
|
||||
"tools.transcription_tools.resolve_openai_audio_api_key", return_value="",
|
||||
), patch(
|
||||
"tools.transcription_tools.resolve_managed_tool_gateway", return_value=None,
|
||||
), patch(
|
||||
"tools.transcription_tools.managed_nous_tools_enabled", return_value=False,
|
||||
):
|
||||
with pytest.raises(ValueError):
|
||||
_resolve_openai_audio_client_config()
|
||||
|
||||
def test_is_local_or_private_url(self):
|
||||
from tools.transcription_tools import _is_local_or_private_url
|
||||
assert _is_local_or_private_url("http://localhost:8504/v1")
|
||||
assert _is_local_or_private_url("http://127.0.0.1:9000")
|
||||
assert _is_local_or_private_url("http://10.0.0.5/v1")
|
||||
assert _is_local_or_private_url("http://stt.internal/v1")
|
||||
assert not _is_local_or_private_url("https://api.openai.com/v1")
|
||||
assert not _is_local_or_private_url("")
|
||||
|
|
|
|||
|
|
@ -35,6 +35,7 @@ import shlex
|
|||
import shutil
|
||||
import subprocess
|
||||
import tempfile
|
||||
import threading
|
||||
from pathlib import Path
|
||||
from typing import Optional, Dict, Any
|
||||
from urllib.parse import urljoin
|
||||
|
|
@ -131,6 +132,10 @@ GROQ_MODELS = {"whisper-large-v3", "whisper-large-v3-turbo", "distil-whisper-lar
|
|||
# Singleton for the local model — loaded once, reused across calls
|
||||
_local_model: Optional[object] = None
|
||||
_local_model_name: Optional[str] = None
|
||||
# Guards the check-then-load of the module-global model cache above.
|
||||
# Without it, two concurrent voice messages can both see `_local_model is
|
||||
# None` and download/load the whisper model twice (#24767).
|
||||
_local_model_lock = threading.Lock()
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Config helpers
|
||||
|
|
@ -1395,20 +1400,24 @@ def _transcribe_local(file_path: str, model_name: str) -> Dict[str, Any]:
|
|||
|
||||
try:
|
||||
local_cfg = _load_stt_config().get("local", {})
|
||||
# Lazy-load the model (downloads on first use, ~150 MB for 'base')
|
||||
# Lazy-load the model (downloads on first use, ~150 MB for 'base').
|
||||
# Double-checked lock: concurrent voice messages must not both
|
||||
# download/load the model (#24767).
|
||||
if _local_model is None or _local_model_name != model_name:
|
||||
logger.info("Loading faster-whisper model '%s' (first load downloads the model)...", model_name)
|
||||
# Honour stt.local.device / stt.local.compute_type from config so
|
||||
# users on hosts where ``auto`` mis-detects (NVIDIA libs present but
|
||||
# not usable, etc.) can pin a working configuration (#9088).
|
||||
# _load_local_whisper_model retains the CUDA→CPU fallback for the
|
||||
# auto/CUDA paths.
|
||||
_local_model = _load_local_whisper_model(
|
||||
model_name,
|
||||
device=local_cfg.get("device", "auto"),
|
||||
compute_type=local_cfg.get("compute_type", "auto"),
|
||||
)
|
||||
_local_model_name = model_name
|
||||
with _local_model_lock:
|
||||
if _local_model is None or _local_model_name != model_name:
|
||||
logger.info("Loading faster-whisper model '%s' (first load downloads the model)...", model_name)
|
||||
# Honour stt.local.device / stt.local.compute_type from config so
|
||||
# users on hosts where ``auto`` mis-detects (NVIDIA libs present but
|
||||
# not usable, etc.) can pin a working configuration (#9088).
|
||||
# _load_local_whisper_model retains the CUDA→CPU fallback for the
|
||||
# auto/CUDA paths.
|
||||
_local_model = _load_local_whisper_model(
|
||||
model_name,
|
||||
device=local_cfg.get("device", "auto"),
|
||||
compute_type=local_cfg.get("compute_type", "auto"),
|
||||
)
|
||||
_local_model_name = model_name
|
||||
|
||||
# Language: stt.local.language > stt.language > env var > auto-detect.
|
||||
stt_config = _load_stt_config()
|
||||
|
|
@ -2204,6 +2213,31 @@ def transcribe_audio(file_path: str, model: Optional[str] = None) -> Dict[str, A
|
|||
shutil.rmtree(cleanup_dir, ignore_errors=True)
|
||||
|
||||
|
||||
def _is_local_or_private_url(url: str) -> bool:
|
||||
"""True when *url* points at a loopback/RFC-1918/LAN-internal host.
|
||||
|
||||
Used to decide whether an empty ``stt.openai.api_key`` is acceptable:
|
||||
local OpenAI-compatible STT servers (faster-whisper-server, speaches,
|
||||
vLLM whisper variants...) ignore the auth header, so users shouldn't
|
||||
have to write a sham ``api_key: not-needed`` in config.yaml.
|
||||
"""
|
||||
try:
|
||||
from urllib.parse import urlparse
|
||||
import ipaddress
|
||||
|
||||
host = (urlparse(url).hostname or "").lower()
|
||||
if not host:
|
||||
return False
|
||||
if host == "localhost" or host.endswith((".local", ".lan", ".internal")):
|
||||
return True
|
||||
try:
|
||||
return ipaddress.ip_address(host).is_private or ipaddress.ip_address(host).is_loopback
|
||||
except ValueError:
|
||||
return False
|
||||
except Exception:
|
||||
return False
|
||||
|
||||
|
||||
def _resolve_openai_audio_client_config() -> tuple[str, str]:
|
||||
"""Return direct OpenAI audio config or a managed gateway fallback."""
|
||||
stt_config = _load_stt_config()
|
||||
|
|
@ -2213,6 +2247,11 @@ def _resolve_openai_audio_client_config() -> tuple[str, str]:
|
|||
if cfg_api_key:
|
||||
return cfg_api_key, (cfg_base_url or OPENAI_BASE_URL)
|
||||
|
||||
# A local OpenAI-compatible server needs no key — send a placeholder so
|
||||
# the SDK doesn't refuse to construct a client (#25193, credit @nnnet).
|
||||
if cfg_base_url and _is_local_or_private_url(cfg_base_url):
|
||||
return "not-needed", cfg_base_url
|
||||
|
||||
direct_api_key = resolve_openai_audio_api_key()
|
||||
if direct_api_key:
|
||||
return direct_api_key, OPENAI_BASE_URL
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue