Merge remote-tracking branch 'origin/master' into HEAD

This commit is contained in:
carlotestor committed 2026-09-23 16:34:03 +00:00
commit 4632ab65e9
29 files changed
+2994 -241

No files matched your search

@@ -0,0 +1,159 @@
"""A wake-up refused for a transient reason the WebUI did not cause must not
consume the durable delivery budget.
Scenario: while the Agent runtime is being replaced, the still-running WebUI
answers every server-side wake-up with ``409 agent_runtime_stale
retryable=True``. Releasing the claim as a plain failure each time exhausts the
bounded budget within seconds and terminally drops a completion whose origin
session is alive and waiting.
Contract under test (``api/background_process._start_async_delegation_wakeup_turn``):
* a refusal whose payload says ``retryable: True`` (stale runtime), a paused
wake-up or a session busy with another stream releases the claim with
``retryable=True`` so the core refunds the attempt;
* every other rejection keeps the historical bounded behaviour;
* an older core whose ``release_event_delivery`` has no ``retryable`` keyword
still gets a plain release (compatibility, never an exception).
"""
from __future__ import annotations
import sys
import time
import types
import pytest
from api import background_process as bp
from api import config as cfg
from api import process_event_utils as peu
def _wait_until(predicate, timeout=3.0):
deadline = time.monotonic() + timeout
while time.monotonic() < deadline:
if predicate():
return True
time.sleep(0.01)
return bool(predicate())
def _reset() -> None:
cfg.PROCESS_SESSION_INDEX.clear()
cfg.PENDING_BG_TASK_COMPLETIONS.clear()
cfg.BG_TASK_COMPLETE_EVENTS_SEEN.clear()
cfg.DEFERRED_PROCESS_WAKEUPS.clear()
reset = getattr(peu, "_reset_legacy_async_delivery_dedupe_for_tests", None)
if reset is not None:
reset()
def _event():
return {
"type": "async_delegation",
"delegation_id": "deleg_retryable",
"session_key": "webui-session-1",
"status": "completed",
"is_batch": True,
"results": [{"task_index": 0, "status": "completed", "summary": "verdict"}],
"dispatched_at": time.time() - 2,
"completed_at": time.time(),
}
class _Registry:
def __init__(self):
import queue
self.completion_queue = queue.Queue()
def _install_core(monkeypatch, *, supports_retryable: bool):
calls = {"release": [], "complete": []}
mod = types.ModuleType("tools.async_delegation")
mod.claim_event_delivery = lambda evt, consumer: f"claim:{consumer}"
mod.complete_event_delivery = lambda evt, claim_id: calls["complete"].append(claim_id)
mod.get_durable_delegation = lambda delegation_id: {"delivery_state": "pending", "delivery_attempts": 1}
mod.restore_undelivered_completions = lambda queue_: 0
if supports_retryable:
def _release(evt, claim_id, *, retryable=False):
calls["release"].append((claim_id, retryable))
else:
def _release(evt, claim_id): # pre-refund core: no keyword at all
calls["release"].append((claim_id, None))
mod.release_event_delivery = _release
pkg = sys.modules.get("tools") or types.ModuleType("tools")
monkeypatch.setitem(sys.modules, "tools", pkg)
monkeypatch.setitem(sys.modules, "tools.async_delegation", mod)
monkeypatch.setattr(pkg, "async_delegation", mod, raising=False)
return calls
def _run(monkeypatch, response, *, supports_retryable=True):
_reset()
calls = _install_core(monkeypatch, supports_retryable=supports_retryable)
from api import routes
monkeypatch.setattr(routes, "start_session_turn", lambda *a, **k: response)
monkeypatch.setattr(bp, "ASYNC_DELIVERY_ROUTING_RETRY_SECONDS", 10.0)
registry = _Registry()
evt = _event()
claim = peu.claim_async_delegation_delivery(evt, "webui-background")
assert claim is not None
try:
bp._start_async_delegation_wakeup_turn(
"webui-session-1", "verdict", delegation_id="deleg_retryable",
evt=evt, claim=claim, process_registry=registry,
)
assert _wait_until(lambda: len(calls["release"]) == 1)
assert calls["complete"] == []
return calls["release"][0]
finally:
_reset()
@pytest.mark.parametrize("response", [
{"_status": 409, "error": "runtime changed", "type": "agent_runtime_stale", "retryable": True},
{"_status": 503, "error": "temporarily unavailable", "retryable": True},
{"_status": 409, "error": "process_wakeup_paused"},
{"_status": 409, "error": "session already has an active stream", "active_stream_id": "s1"},
])
def test_transient_refusal_releases_with_refund(monkeypatch, response):
_claim, retryable = _run(monkeypatch, response)
assert retryable is True
@pytest.mark.parametrize("response", [
{"_status": 500, "error": "boom"},
{"_status": 404, "error": "Session not found"},
{"_status": 400, "error": "message is required"},
{"_status": 409, "error": "conflict", "retryable": False},
None,
])
def test_hard_rejection_keeps_bounded_budget(monkeypatch, response):
_claim, retryable = _run(monkeypatch, response)
assert retryable is False
def test_old_core_without_retryable_keyword_still_releases(monkeypatch):
_claim, retryable = _run(
monkeypatch,
{"_status": 409, "error": "x", "type": "agent_runtime_stale", "retryable": True},
supports_retryable=False,
)
assert retryable is None # plain release happened, no TypeError escaped
def test_real_core_signature_is_detected():
def new_core(evt, claim_id, *, retryable=False):
return None
def kwargs_core(evt, claim_id, **kwargs):
return None
def old_core(evt, claim_id):
return None
assert peu._release_accepts_retryable(new_core) is True
assert peu._release_accepts_retryable(kwargs_core) is True
assert peu._release_accepts_retryable(old_core) is False
@@ -1,5 +1,6 @@
"""Regression coverage for notify_on_complete across a WebUI restart."""
import logging
import threading
import time
from types import SimpleNamespace
@@ -158,4 +159,103 @@ def test_recovery_is_fail_soft_without_agent(monkeypatch):
monkeypatch.setattr("builtins.__import__", fake_import)
assert bp.recover_processes_for_webui() == 0
assert bp._PROCESS_RECOVERY_DONE is False
assert bp._PROCESS_RECOVERY_DONE is False
def test_recovery_skips_process_whose_session_is_gone(monkeypatch, caplog):
"""A checkpointed process whose owner has been pruned (the resolver's
``KeyError(sid)`` contract) must be silently skipped, while a neighbouring
live process in the same recovery sweep still rebinds. Regression for
issue #7753: previously every vanished owner logged a full WARNING
traceback, indistinguishable from a real fault.
"""
calls = {"registered": []}
class FakeRegistry:
def recover_from_checkpoint(self):
return 2 # two checkpointed processes survive restart
def list_sessions(self):
return [
{"session_id": "proc_vanished", "detached": True},
{"session_id": "proc_live", "detached": True},
]
def get(self, process_id):
if process_id == "proc_vanished":
return _FakeProcessSession("vanished-session")
if process_id == "proc_live":
return _FakeProcessSession("live-session")
raise AssertionError(process_id)
fake_registry = FakeRegistry()
monkeypatch.setattr(bp, "_PROCESS_CHECKPOINT_RECOVERED", False)
monkeypatch.setattr(bp, "_PROCESS_RECOVERY_DONE", False)
monkeypatch.setattr(
bp,
"register_process_session",
lambda key, sid: calls["registered"].append((key, sid)),
)
def get_session(sid, metadata_only=False):
# Mirrors api.models._resolve_session_once's contract: missing
# sessions raise KeyError, NOT None.
if sid == "vanished-session":
raise KeyError(sid)
return SimpleNamespace(id=sid)
with caplog.at_level(logging.WARNING, logger="api.background_process"):
result = bp.recover_processes_for_webui(fake_registry, get_session)
assert result == 2
# The live process is rebound; the vanished one is silently skipped.
assert calls["registered"] == [("live-session", "live-session")]
# No WARNING emitted for the vanished-owner case.
warning_records = [
r for r in caplog.records if r.levelno >= logging.WARNING
]
assert warning_records == [], (
"vanished-session KeyError must not produce a WARNING, got: "
f"{[r.getMessage() for r in warning_records]}"
)
assert "Could not resolve recovered WebUI process" not in caplog.text
def test_recovery_still_warns_on_unexpected_resolve_failure(monkeypatch, caplog):
"""Non-KeyError failures from the resolver (e.g. ``OSError`` on an
unreadable sidecar) must keep entering the existing WARNING path. This
pins that the fix for #7753 is narrow and does not over-broaden the
silent-skip contract.
"""
class FakeRegistry:
def recover_from_checkpoint(self):
return 1
def list_sessions(self):
return [{"session_id": "proc_unexpected", "detached": True}]
def get(self, process_id):
assert process_id == "proc_unexpected"
return _FakeProcessSession("webui-session")
fake_registry = FakeRegistry()
monkeypatch.setattr(bp, "_PROCESS_CHECKPOINT_RECOVERED", False)
monkeypatch.setattr(bp, "_PROCESS_RECOVERY_DONE", False)
def get_session(sid, metadata_only=False):
raise OSError("sidecar unreadable")
with caplog.at_level(logging.WARNING, logger="api.background_process"):
result = bp.recover_processes_for_webui(fake_registry, get_session)
assert result == 1
warning_records = [
r for r in caplog.records if r.levelno >= logging.WARNING
]
assert any(
"Could not resolve recovered WebUI process 'proc_unexpected'" in r.getMessage()
for r in warning_records
), (
"OSError from the resolver must still emit a WARNING, got: "
f"{[r.getMessage() for r in warning_records]}"
)
+8 -1
View File
@@ -291,6 +291,13 @@ class TestLiveModelsCustomProviderFallback:
"model": "gpt-5.5",
"models": {"gpt-5.5-mini": {}},
"base_url": "https://right.codes/codex/v1",
# dict-shaped ``models`` is per-model metadata (Agent setup
# flow) and only narrows the live catalog when discovery is
# explicitly off — the new #7165 contract. This test still
# wants provider-scoping (no sibling leak), so pin the
# catalog via ``discover_models: false`` instead of relying
# on the removed dict-as-allowlist behavior.
"discover_models": False,
},
{
"name": "infini-ai",
@@ -308,7 +315,7 @@ class TestLiveModelsCustomProviderFallback:
resp = self._call_live_models(monkeypatch, cfg, "custom:rightcode-codex")
assert resp["provider"] == "custom:rightcode-codex"
assert [m["id"] for m in resp["models"]] == ["gpt-5.5", "gpt-5.5-mini"]
assert [m["id"] for m in resp["models"]] == ["gpt-5.5-mini"]
def test_bare_custom_fallback_ignores_named_custom_provider_models(self, monkeypatch):
"""Bare custom only represents unnamed custom entries, not named siblings."""
@@ -0,0 +1,222 @@
"""Regression: a re-subscribing SSE client gets a fresh watcher projection.
``_poll_once`` resets the parity timestamp when it runs with no subscribers
("force fresh projection on the next subscription"), but since the idle-park
change (#7694) the poll loop parks *before* calling ``_poll_once`` once the
last subscriber leaves, so that reset is not reached on a normal disconnect.
A tab that reconnects inside the parity interval then resumes with the stale
cheap fingerprint and skips the projection. Final removal invalidates the
cache, and an epoch fences projections already in flight.
"""
from __future__ import annotations
import importlib
import sqlite3
import threading
import time
from pathlib import Path
import pytest
def _make_db(tmp_path: Path) -> Path:
db = tmp_path / "state.db"
conn = sqlite3.connect(str(db))
conn.executescript(
"""
CREATE TABLE sessions (
id TEXT PRIMARY KEY,
source TEXT NOT NULL,
session_source TEXT,
model TEXT,
started_at REAL NOT NULL,
ended_at REAL,
end_reason TEXT,
parent_session_id TEXT,
message_count INTEGER DEFAULT 0,
title TEXT,
archived INTEGER DEFAULT 0
);
CREATE TABLE messages (
id INTEGER PRIMARY KEY AUTOINCREMENT,
session_id TEXT NOT NULL,
role TEXT NOT NULL,
content TEXT,
timestamp REAL NOT NULL
);
"""
)
conn.execute(
"INSERT INTO sessions (id, source, model, started_at, message_count, title) "
"VALUES ('tg1', 'telegram', 'm', ?, 1, 'Chat')",
(time.time(),),
)
conn.execute(
"INSERT INTO messages (session_id, role, content, timestamp) VALUES ('tg1', 'user', 'x', ?)",
(time.time(),),
)
conn.commit()
conn.close()
return db
def test_resubscribe_after_last_unsubscribe_forces_fresh_projection(tmp_path):
gw = importlib.import_module("api.gateway_watcher")
watcher = gw.GatewayWatcher(state_db_path=_make_db(tmp_path))
q = watcher.subscribe()
assert watcher._poll_once(now=1.0) is True
watcher.unsubscribe(q)
assert watcher._last_cheap_fp == ""
assert watcher._last_full_projection_at is None
watcher.subscribe()
# Well inside PROJECTION_PARITY_INTERVAL and with an unchanged state.db.
assert watcher._poll_once(now=2.0) is True
def test_unsubscribe_keeps_fingerprint_while_other_subscribers_remain(tmp_path):
gw = importlib.import_module("api.gateway_watcher")
watcher = gw.GatewayWatcher(state_db_path=_make_db(tmp_path))
first = watcher.subscribe()
watcher.subscribe()
assert watcher._poll_once(now=1.0) is True
fingerprint = watcher._last_cheap_fp
assert fingerprint
watcher.unsubscribe(first)
assert watcher._last_cheap_fp == fingerprint
assert watcher._last_full_projection_at == 1.0
assert watcher._poll_once(now=2.0) is False
def test_unsubscribe_of_unknown_queue_does_not_reset(tmp_path):
import queue as _queue
gw = importlib.import_module("api.gateway_watcher")
watcher = gw.GatewayWatcher(state_db_path=_make_db(tmp_path))
watcher.subscribe()
assert watcher._poll_once(now=1.0) is True
fingerprint = watcher._last_cheap_fp
watcher.unsubscribe(_queue.Queue())
assert watcher._last_cheap_fp == fingerprint
assert watcher._last_full_projection_at == 1.0
def test_slow_consumer_eviction_invalidates_populated_cache(tmp_path):
gw = importlib.import_module("api.gateway_watcher")
watcher = gw.GatewayWatcher(state_db_path=_make_db(tmp_path))
q = watcher.subscribe()
assert watcher._poll_once(now=1.0) is True
assert watcher._last_cheap_fp
assert watcher._last_full_projection_at == 1.0
for _ in range(q.maxsize - q.qsize()):
q.put_nowait({"type": "existing"})
watcher._notify_subscribers([])
assert not watcher._has_subscribers()
assert watcher._last_cheap_fp == ""
assert watcher._last_full_projection_at is None
watcher.unsubscribe(q) # The late SSE cleanup cannot undo the reset.
watcher.subscribe()
assert watcher._poll_once(now=2.0) is True
@pytest.mark.parametrize("removal", ["unsubscribe", "slow_consumer"])
@pytest.mark.parametrize("reconnect_while_blocked", [False, True])
def test_inflight_projection_cannot_restore_cache_after_last_removal(
tmp_path, monkeypatch, removal, reconnect_while_blocked,
):
"""The DB read must not serialize final removal or commit across its epoch."""
gw = importlib.import_module("api.gateway_watcher")
watcher = gw.GatewayWatcher(state_db_path=_make_db(tmp_path))
q = watcher.subscribe()
entered = threading.Event()
release = threading.Event()
projections = []
errors = []
real_projection = gw._get_agent_sessions_from_db
def blocked_projection(path):
projections.append(path)
if len(projections) == 1:
entered.set()
assert release.wait(5), "test never released the DB read"
return real_projection(path)
def poll():
try:
watcher._poll_once(now=1.0)
except BaseException as exc:
errors.append(exc)
monkeypatch.setattr(gw, "_get_agent_sessions_from_db", blocked_projection)
worker = threading.Thread(target=poll, daemon=True)
worker.start()
new_q = None
try:
assert entered.wait(5), "poll never reached the DB read"
if removal == "unsubscribe":
watcher.unsubscribe(q)
else:
for _ in range(q.maxsize):
q.put_nowait({"type": "existing"})
watcher._notify_subscribers([])
# Handler teardown of an evicted queue must be harmless.
watcher.unsubscribe(q)
assert not watcher._has_subscribers()
assert watcher._last_cheap_fp == ""
assert watcher._last_full_projection_at is None
if reconnect_while_blocked:
new_q = watcher.subscribe()
finally:
release.set()
worker.join(timeout=5)
assert not worker.is_alive(), "poll did not finish"
assert not errors, errors
assert watcher._last_cheap_fp == ""
assert watcher._last_full_projection_at is None
if new_q is not None:
assert new_q.empty(), "old projection must not notify a new subscriber"
else:
watcher.subscribe()
assert watcher._poll_once(now=2.0) is True
assert len(projections) == 2
assert watcher._last_cheap_fp
assert watcher._last_full_projection_at == 2.0
def test_poll_loop_reprojects_for_reconnecting_subscriber(tmp_path, monkeypatch):
"""Through the real loop: disconnect, park, reconnect -> projection runs again."""
gw = importlib.import_module("api.gateway_watcher")
db = _make_db(tmp_path)
projections: list[float] = []
real_projection = gw._get_agent_sessions_from_db
def tracing_projection(path):
projections.append(time.monotonic())
return real_projection(path)
monkeypatch.setattr(gw, "_get_agent_sessions_from_db", tracing_projection)
watcher = gw.GatewayWatcher(state_db_path=db)
watcher.POLL_INTERVAL = 0.05
watcher.start()
try:
q = watcher.subscribe()
deadline = time.monotonic() + 2.0
while not projections and time.monotonic() < deadline:
time.sleep(0.01)
assert len(projections) == 1
watcher.unsubscribe(q)
time.sleep(watcher.POLL_INTERVAL * 4) # loop parks; no further projection
assert len(projections) == 1
watcher.subscribe()
deadline = time.monotonic() + 2.0
while len(projections) < 2 and time.monotonic() < deadline:
time.sleep(0.01)
assert len(projections) == 2, "reconnecting subscriber must get a fresh projection"
finally:
watcher.stop()
+390 -185
View File
@@ -1,193 +1,398 @@
"""Regression test for #3718 -- /api/models/live skips probe for custom providers.
"""Handler-level regression tests for /api/models/live custom-provider probing.
When a custom provider (``custom_providers`` entry in config.yaml) has a
``model:`` field but no ``models:`` allowlist, the live endpoint previously
populated ``ids`` from the config entry and then skipped the live ``/v1/models``
probe (guarded by ``if not ids``). The fix collects config-specified model IDs
in a separate ``_config_ids`` list so the live fetch always runs, and merges
config entries as a fallback after the fetch.
These tests invoke the real ``routes._handle_live_models`` and inspect the
emitted ``models`` payload. Earlier revisions of this file re-implemented the
filtering loop locally, which meant they passed unchanged even with the
production filter deleted (a vacuous oracle). Every case here must fail if the
allowlist gating in ``api/routes.py`` regresses.
Covered shapes:
* native ``models`` list → live catalog filtered
* JSON-array-string ``models`` → live catalog filtered (#7120 class)
* Python-literal-string ``models`` → live catalog filtered
* singular ``model`` only, no ``models`` → live catalog NOT gated
* malformed serialized ``models`` → safe, treated as scalar name
* allowlisted model absent from live catalog → appended
* live probe failure → falls back to config ids
* dict-shaped ``models`` (Agent setup) → live catalog NOT gated (per-model
metadata, not an allowlist)
* dict-shaped ``models`` + ``discover_models:
false`` → pinned to dict keys (explicit opt-out)
* ``models_discovered: true`` catalog → live catalog NOT gated (discovery
metadata is not an allowlist)
* discovered + ``discover_models: false`` → still pinned (explicit opt-out)
"""
import io
import json
import pathlib
import unittest
from unittest import mock
import sys
import types
from urllib.parse import urlparse
REPO = pathlib.Path(__file__).parent.parent
ROUTES_PY = REPO / "api" / "routes.py"
import pytest
class TestLiveModelsCustomProviderProbe(unittest.TestCase):
"""Live endpoint must probe the upstream /v1/models even when config has entries."""
def test_custom_provider_with_model_field_probes_upstream(self):
"""When a custom provider has model: in config, live fetch must still run.
Before the fix, _custom_provider_model_ids() populated ids=["assistant"],
and the `if not ids` guard prevented the upstream /v1/models probe from
running. The endpoint returned only ["assistant"] instead of the full
upstream catalog.
We test the fix by simulating the logic directly: config provides
model IDs, the upstream probe returns a different set, and the result
must contain BOTH the live models and the config fallback.
"""
# Simulate config-specified model IDs
_config_ids = ["assistant"]
# Simulate upstream /v1/models response
live_models = [
"assistant",
"assistant-pro",
"assistant-zdr",
"gpt-5.5:pt",
"claude-sonnet-4.6:pt",
]
# Simulate the merge logic from the fix:
# if ids: merge config entries not in live set
# else: fall back to config-only list
ids = list(live_models) # live fetch succeeded
_live_set = set(ids)
for _cid in _config_ids:
if _cid not in _live_set:
ids.append(_cid)
# "assistant" from config should not duplicate the live result
self.assertEqual(ids, live_models)
# All live models must be present
for m in live_models:
self.assertIn(m, ids)
def test_custom_provider_falls_back_to_config_when_probe_fails(self):
"""When the upstream probe fails, config entries must be used as fallback."""
_config_ids = ["assistant", "custom-only-model"]
# Simulate failed live fetch: ids stays empty
ids = []
# Fallback logic from the fix
if not ids:
ids = list(_config_ids)
self.assertEqual(ids, ["assistant", "custom-only-model"])
def test_config_only_model_appended_when_not_in_live_results(self):
"""Config-specified models not in the live results must be appended."""
_config_ids = ["assistant", "local-finetune"]
live_models = ["assistant", "assistant-pro", "gpt-5.5:pt"]
ids = list(live_models)
_live_set = set(ids)
for _cid in _config_ids:
if _cid not in _live_set:
ids.append(_cid)
# "assistant" is in both, "local-finetune" is config-only
self.assertIn("local-finetune", ids)
self.assertIn("assistant-pro", ids)
self.assertEqual(ids.count("assistant"), 1, "assistant must not be duplicated")
def test_live_fetch_guard_no_longer_checks_ids(self):
"""The live fetch block must not be guarded by `if not ids`.
This is a structural regression guard: the fix replaced
`if not ids and (provider == "custom" ...)` with
`if provider == "custom" ...` so the probe always runs regardless
of whether config entries populated ids.
"""
source = ROUTES_PY.read_text(encoding="utf-8")
# Find the "Always try live fetch" comment (added by the fix)
marker = "Always try live fetch for custom providers"
self.assertIn(marker, source, (
"routes.py must contain the 'Always try live fetch' comment (#3718)"
))
# Extract the block between the marker and the next major section
marker_pos = source.find(marker)
next_section = source.find("OpenAI-compat live fetch fallback", marker_pos)
self.assertNotEqual(next_section, -1, "could not find next section marker")
block = source[marker_pos:next_section]
# The old `if not ids and` guard must NOT appear in this block
self.assertNotIn("if not ids and", block, (
"Live fetch for custom providers must not be guarded by 'if not ids' (#3718)"
))
def test_mocked_live_fetch_returns_full_catalog_plus_config_entry(self):
"""Integration test: mock urlopen and exercise the real fetch/parse/merge path.
This test patches urllib.request.urlopen to return a fake /v1/models
response, then runs the same fetch+merge logic that the live handler
uses, verifying that config entries are merged and live models are
included.
"""
fake_models = {"data": [
{"id": "assistant"},
{"id": "assistant-pro"},
{"id": "gpt-5.5:pt"},
]}
fake_body = json.dumps(fake_models).encode("utf-8")
fake_resp = mock.MagicMock()
fake_resp.read.return_value = fake_body
fake_resp.__enter__ = mock.MagicMock(return_value=fake_resp)
fake_resp.__exit__ = mock.MagicMock(return_value=False)
_config_ids = ["assistant", "local-only"]
with mock.patch("urllib.request.urlopen", return_value=fake_resp):
# Replicate the fetch+parse logic from routes.py
import urllib.request
ids = []
_req = urllib.request.Request(
"http://localhost:4000/v1/models",
headers={"Authorization": "Bearer test-key"},
)
with urllib.request.urlopen(_req, timeout=5) as _resp:
_body = json.loads(_resp.read())
if isinstance(_body, dict):
_data = _body.get("data", [])
if isinstance(_data, list):
ids = [m.get("id", "") for m in _data if m.get("id")]
# Apply the merge logic from the fix
if ids:
_live_set = set(ids)
for _cid in _config_ids:
if _cid not in _live_set:
ids.append(_cid)
else:
ids = list(_config_ids)
# Live models must all be present
self.assertIn("assistant", ids)
self.assertIn("assistant-pro", ids)
self.assertIn("gpt-5.5:pt", ids)
# Config-only entry must be appended
self.assertIn("local-only", ids)
# No duplicates
self.assertEqual(ids.count("assistant"), 1)
def test_timeout_uses_config_constant(self):
"""The live fetch must use CUSTOM_MODELS_ENDPOINT_TIMEOUT_SECONDS, not a hardcoded value."""
source = ROUTES_PY.read_text(encoding="utf-8")
# Find the custom-provider live fetch block
marker = "Always try live fetch for custom providers"
marker_pos = source.find(marker)
next_section = source.find("OpenAI-compat live fetch fallback", marker_pos)
block = source[marker_pos:next_section]
# Must use the constant, not a bare number
self.assertIn("CUSTOM_MODELS_ENDPOINT_TIMEOUT_SECONDS", block, (
"Live fetch timeout must use CUSTOM_MODELS_ENDPOINT_TIMEOUT_SECONDS "
"instead of a hardcoded value (#3718 review feedback)"
))
self.assertNotIn("timeout=8", block, (
"Live fetch must not use hardcoded timeout=8 (#3718 review feedback)"
))
def _install_provider_model_ids(monkeypatch, fn):
"""Stub hermes_cli.models.provider_model_ids (returns [] for custom:*)."""
hermes_cli = types.ModuleType("hermes_cli")
hermes_cli.__path__ = []
models = types.ModuleType("hermes_cli.models")
models.provider_model_ids = fn
monkeypatch.setitem(sys.modules, "hermes_cli", hermes_cli)
monkeypatch.setitem(sys.modules, "hermes_cli.models", models)
if __name__ == "__main__":
unittest.main()
class _FakeResponse(io.BytesIO):
def __enter__(self):
return self
def __exit__(self, *_exc):
self.close()
return False
def _install_live_catalog(monkeypatch, routes, catalog, *, fail=False):
"""Stub the upstream /v1/models probe used by the custom-provider branch."""
import urllib.request
requested: list[str] = []
def _fake_urlopen(req, timeout=None):
requested.append(getattr(req, "full_url", str(req)))
if fail:
raise OSError("probe failed")
body = {"data": [{"id": mid} for mid in catalog]}
return _FakeResponse(json.dumps(body).encode())
monkeypatch.setattr(urllib.request, "urlopen", _fake_urlopen)
return requested
def _run_live_models(monkeypatch, routes, custom_provider, *, catalog, fail=False):
"""Invoke the real handler for provider=custom:test-gateway."""
import api.config as config
import api.profiles as profiles
routes._clear_live_models_cache()
monkeypatch.setattr(
routes, "j", lambda _handler, payload, status=200, extra_headers=None: payload
)
monkeypatch.setattr(config, "_resolve_provider_alias", lambda provider: provider)
monkeypatch.setattr(profiles, "get_active_profile_name", lambda: "default")
config_data = {
"model": {"provider": "custom:test-gateway"},
"custom_providers": [custom_provider],
}
monkeypatch.setattr(config, "get_config", lambda: config_data)
monkeypatch.setattr(routes, "cfg", config_data, raising=False)
# provider_model_ids() returns [] for custom:* → drives the custom branch.
_install_provider_model_ids(monkeypatch, lambda provider: [])
requested = _install_live_catalog(monkeypatch, routes, catalog, fail=fail)
parsed = urlparse("/api/models/live?provider=custom:test-gateway")
payload = routes._handle_live_models(object(), parsed)
return payload, requested
def _ids(payload):
return [m["id"] for m in payload.get("models", [])]
_LIVE_CATALOG = [
"chat-a",
"chat-b",
"image-gen-1", # non-chat noise the picker must not surface
"audio-tts-1",
]
_BASE_PROVIDER = {
"name": "Test Gateway",
"base_url": "https://gateway.example/v1",
"api_key": "sk-test",
}
@pytest.mark.parametrize(
"models_value",
[
pytest.param(["chat-a", "chat-b"], id="native-list"),
pytest.param('["chat-a","chat-b"]', id="json-array-string"),
pytest.param("['chat-a', 'chat-b']", id="python-literal-string"),
pytest.param(
[{"id": "chat-a"}, {"model": "chat-b"}], id="list-of-dicts"
),
],
)
def test_allowlist_filters_live_catalog(monkeypatch, models_value):
"""An explicit plural allowlist must trim the live catalog to itself.
The JSON-array-string and Python-literal cases are the regression: those
shapes are what ``hermes config set`` / JSON-mode editor saves persist, and
a dict/list-only parser drops them to ``[]`` → read as "no allowlist" → the
full upstream catalog floods the picker.
"""
import api.routes as routes
provider = dict(_BASE_PROVIDER, models=models_value)
payload, requested = _run_live_models(
monkeypatch, routes, provider, catalog=_LIVE_CATALOG
)
assert requested, "live /v1/models probe was never attempted"
assert _ids(payload) == ["chat-a", "chat-b"]
assert "image-gen-1" not in _ids(payload)
assert "audio-tts-1" not in _ids(payload)
def test_dict_models_without_discover_false_does_not_gate(monkeypatch):
"""A dict-shaped ``models`` is per-model metadata — NOT an allowlist (#7165).
Hermes Agent's setup flow and ``hermes_cli/model_switch.py::_save_custom_provider``
persist per-model metadata as a mapping, e.g. ``{chat-a: {context_length: 128000}}``.
Treating its keys as an allowlist would collapse the live picker to the saved
default while the CLI live-probe shows the full catalog. Without an explicit
``discover_models: false`` opt-out the dict must NOT gate — full live catalog wins.
"""
import api.routes as routes
provider = dict(
_BASE_PROVIDER,
models={"chat-a": {"context_length": 128000}, "chat-b": {"context_length": 32000}},
)
payload, requested = _run_live_models(
monkeypatch, routes, provider, catalog=_LIVE_CATALOG
)
assert requested, "live /v1/models probe was never attempted"
assert _ids(payload) == _LIVE_CATALOG, (
"dict-shaped models is per-model metadata, not an allowlist — the "
"full live catalog must be surfaced"
)
def test_dict_models_with_discover_false_pins_to_keys(monkeypatch):
"""``discover_models: false`` opts a dict-shaped ``models`` into an allowlist.
The explicit opt-out (bool ``False`` or the string forms ``"false"``/``"no"``/``"0"``)
means the dict keys are a hand-pinned allowlist and must filter the live catalog.
"""
import api.routes as routes
for discover in (False, "false", "no", "0"):
provider = dict(
_BASE_PROVIDER,
models={"chat-a": {"context_length": 128000}, "chat-b": {"context_length": 32000}},
discover_models=discover,
)
payload, requested = _run_live_models(
monkeypatch, routes, provider, catalog=_LIVE_CATALOG
)
assert requested, "live /v1/models probe was never attempted"
assert _ids(payload) == ["chat-a", "chat-b"], (
f"discover_models={discover!r} must pin the dict keys as the allowlist"
)
assert "image-gen-1" not in _ids(payload)
assert "audio-tts-1" not in _ids(payload)
def test_singular_model_only_does_not_gate_live_catalog(monkeypatch):
"""``model:`` is default metadata, not an allowlist — keep full discovery."""
import api.routes as routes
provider = dict(_BASE_PROVIDER, model="chat-a")
payload, _ = _run_live_models(
monkeypatch, routes, provider, catalog=_LIVE_CATALOG
)
assert _ids(payload) == _LIVE_CATALOG
def test_malformed_serialized_models_is_safe(monkeypatch):
"""A broken JSON-array string must not crash or silently drop the gate.
``_parse_config_string_list()`` treats an undecodable value as one literal
name, so the allowlist stays non-empty and the filter still runs.
"""
import api.routes as routes
provider = dict(_BASE_PROVIDER, models='["chat-a", "chat-b"') # missing ]
payload, _ = _run_live_models(
monkeypatch, routes, provider, catalog=_LIVE_CATALOG
)
# Treated as a single (unmatched) name → appended, catalog not flooded.
assert "image-gen-1" not in _ids(payload)
assert "audio-tts-1" not in _ids(payload)
def test_allowlisted_model_missing_from_live_is_appended(monkeypatch):
"""A declared model the upstream didn't return must still be selectable."""
import api.routes as routes
provider = dict(_BASE_PROVIDER, models=["chat-a", "chat-offline"])
payload, _ = _run_live_models(
monkeypatch, routes, provider, catalog=_LIVE_CATALOG
)
assert _ids(payload) == ["chat-a", "chat-offline"]
def test_contradictory_singular_model_outside_plural_allowlist_is_ignored(monkeypatch):
"""A singular default outside the plural allowlist must not leak into the picker.
Paired test: whether the live probe succeeds or fails, only the explicit
plural allowlist is exposed.
"""
import api.routes as routes
provider = dict(_BASE_PROVIDER, model="chat-default", models=["chat-a"])
# 1. Successful live fetch -> only plural allowlist
payload_success, _ = _run_live_models(
monkeypatch, routes, provider, catalog=_LIVE_CATALOG, fail=False
)
assert _ids(payload_success) == ["chat-a"]
# 2. Failed live fetch -> still only plural allowlist (not chat-default)
payload_fail, _ = _run_live_models(
monkeypatch, routes, provider, catalog=_LIVE_CATALOG, fail=True
)
assert _ids(payload_fail) == ["chat-a"]
def test_probe_failure_without_allowlist_falls_back_to_singular_model(monkeypatch):
"""When no plural allowlist is configured and probe fails, singular model is used."""
import api.routes as routes
provider = dict(_BASE_PROVIDER, model="chat-default")
payload, _ = _run_live_models(
monkeypatch, routes, provider, catalog=_LIVE_CATALOG, fail=True
)
assert _ids(payload) == ["chat-default"]
@pytest.mark.parametrize(
"models_value",
[
pytest.param([], id="native-empty-list"),
pytest.param("[]", id="serialized-empty-list"),
pytest.param({}, id="empty-mapping"),
pytest.param(" ", id="blank-scalar"),
],
)
def test_empty_allowlist_is_treated_as_not_configured(monkeypatch, models_value):
"""An empty ``models`` value must NOT be read as "allow nothing".
Pins a deliberate semantic decision. Gating the filter on *declared-ness*
rather than on a non-empty allowlist would make ``models: []`` emit an
EMPTY picker, leaving the provider unselectable — a harder failure than
surfacing a few extra models, and unrecoverable from the UI because
``custom_providers`` has no WebUI write path (hand-edited config.yaml only).
There is also no deny-all use case: a provider the user wants hidden is
removed from ``custom_providers`` outright.
If someone later "fixes" the empty case into a deny-all gate, this test
fails and forces the trade-off to be re-argued rather than silently shipped.
"""
import api.routes as routes
provider = dict(_BASE_PROVIDER, models=models_value)
payload, _ = _run_live_models(
monkeypatch, routes, provider, catalog=_LIVE_CATALOG
)
assert _ids(payload) == _LIVE_CATALOG, (
"empty allowlist must fall through to full live discovery, "
"never collapse the picker to zero models"
)
assert _ids(payload), "picker must never be emptied by an empty allowlist"
def test_models_discovered_catalog_is_not_an_allowlist(monkeypatch):
"""``models_discovered: true`` marks ``models:`` as discovery metadata (#7165).
Hermes persists *discovery results* back into config as ``models: {...}``
plus ``models_discovered: true``. That mapping is a snapshot of what the
gateway exposed at discovery time — NOT a hand-curated allowlist. Treating
it as one permanently pins the live catalog to the first-discovery set, so
a model the user later pulls into LM Studio / Ollama is silently dropped
from ``/api/models/live`` (the exact regression the gate reproduced: merge
base returned ``saved-a``, ``saved-b`` AND ``new-live-c``; the pinned head
dropped ``new-live-c``).
"""
import api.routes as routes
provider = dict(
_BASE_PROVIDER,
models={
"saved-a": {"context_length": 128000},
"saved-b": {"context_length": 32000},
},
models_discovered=True,
)
payload, requested = _run_live_models(
monkeypatch,
routes,
provider,
catalog=["saved-a", "saved-b", "new-live-c"],
)
assert requested, "live /v1/models probe was never attempted"
assert _ids(payload) == ["saved-a", "saved-b", "new-live-c"], (
"a discovered catalog must not gate the live probe — models that "
"appear upstream after discovery (new-live-c) must reach the picker"
)
def test_discovered_catalog_with_discover_models_false_still_pins(monkeypatch):
"""``discover_models: false`` re-pins a discovered catalog (control).
The explicit opt-out wins over ``models_discovered: true`` — same rule as
the ``/api/models`` catalog path (`api.config._provider_discover_allowed`).
Filtering to the configured set must survive the discovered-catalog fix.
"""
import api.routes as routes
provider = dict(
_BASE_PROVIDER,
models={
"saved-a": {"context_length": 128000},
"saved-b": {"context_length": 32000},
},
models_discovered=True,
discover_models=False,
)
payload, _ = _run_live_models(
monkeypatch,
routes,
provider,
catalog=["saved-a", "saved-b", "new-live-c"],
)
assert _ids(payload) == ["saved-a", "saved-b"]
def test_discovered_catalog_probe_failure_falls_back_to_saved_models(monkeypatch):
"""Discovered + failed probe → saved models, not an empty picker (control).
Releasing the allowlist must not empty the fallback: when the live probe
fails, the previously discovered ``models:`` mapping is still the best
answer and must be served from ``_config_ids``.
"""
import api.routes as routes
provider = dict(
_BASE_PROVIDER,
models={
"saved-a": {"context_length": 128000},
"saved-b": {"context_length": 32000},
},
models_discovered=True,
)
payload, _ = _run_live_models(
monkeypatch, routes, provider, catalog=[], fail=True
)
assert _ids(payload) == ["saved-a", "saved-b"]
@@ -0,0 +1,313 @@
"""Regression tests for issue #7359 — MEDIA: token regex must not capture trailing backtick.
Two-pass scan (#7680 re-gate, 9/22):
- `` `MEDIA:path` `` (backtick-wrapped, inline-code form): the wrapping
backticks are stripped by a pre-pass before the bare class scans the
text. This makes the closing backtick disappear as a wrapper instead
of being captured as part of the path.
- ``MEDIA:/path...`` (bare, no surrounding backticks): the capture class
no longer lists the backtick in the exclusion set, so a filename
that legally contains a backtick (``report`final.png``) is captured
in full instead of being truncated at the first backtick.
The pre-fix (round-1) PR added the backtick to the bare exclusion set,
which traded one bug for another: it dropped the closing wrapper on
the inline-code form but truncated any bare path that happened to
contain a backtick in the filename. The two-pass scan keeps the
original permissive bare class and adds an explicit pre-pass for the
wrapped form, so both cases round-trip correctly.
"""
import re
import subprocess
from pathlib import Path
import pytest
REPO_ROOT = Path(__file__).resolve().parents[1]
# Source-shape: each site's expected bare-token capture literal. The
# bare class no longer lists the backtick in the exclusion set; a
# backtick inside the filename (legal on POSIX and Windows) is
# captured in full. The wrapped-form pre-pass below strips the
# inline-code backticks before the bare scan.
JS_SITES: dict[str, list[str]] = {
"static/ui.js": [
r"/MEDIA:([^\s\)\]]+)/g", # linkifier
r"/MEDIA:[^\s]+/g", # TTS flattener (no paren/bracket exclusion)
],
"static/messages.js": [
r"/^MEDIA:([^\s\)\]]+)$/", # streaming chunk finalize
r"/MEDIA:([^\s\)\]]+)/g", # streaming combined walk
r"/MEDIA:[^\s\)\]]*$/", # streaming partial tail
],
}
PY_SITES: dict[str, str] = {
"api/routes.py": r"MEDIA:([^\s\)\]]+)",
"api/media_snapshots.py": r"MEDIA:([^\s\)\]]+)",
}
# #7680 re-gate (9/22): every site that scans bare MEDIA: tokens also
# runs the wrapped-form pre-pass (`` `MEDIA:path` `` → ``MEDIA:path``)
# so the closing backtick of an inline-code form is consumed before the
# bare scan.
JS_BACKTICK_PREPASS: dict[str, str] = {
"static/ui.js": r"/`MEDIA:([^`\s]+)`/g",
"static/messages.js": r"/`MEDIA:([^`\s]+)`/g",
}
PY_BACKTICK_PREPASS: dict[str, str] = {
"api/routes.py": r"`MEDIA:([^`\s]+)`",
"api/media_snapshots.py": r"`MEDIA:([^`\s]+)`",
}
def _read(rel: str) -> str:
return (REPO_ROOT / rel).read_text(encoding="utf-8")
# ---------------------------------------------------------------------------
# Source-shape: every site must carry the bare-token class + the pre-pass
# ---------------------------------------------------------------------------
@pytest.mark.parametrize(
("relpath", "needle"),
[
(relpath, needle)
for relpath, needles in JS_SITES.items()
for needle in needles
],
)
def test_js_site_carries_bare_token_class(relpath: str, needle: str) -> None:
"""Every JS capture site carries the bare-token class literal."""
src = _read(relpath)
assert needle in src, (
f"{relpath} is missing the bare-token capture literal: {needle!r}"
)
@pytest.mark.parametrize(
("relpath", "needle"),
[
(relpath, JS_BACKTICK_PREPASS[relpath])
for relpath in JS_BACKTICK_PREPASS
],
)
def test_js_site_carries_wrapped_prepass(relpath: str, needle: str) -> None:
"""#7680 re-gate: every JS site that scans bare MEDIA: tokens also
runs the wrapped-form pre-pass so the inline-code closing backtick
is eaten before the bare scan."""
src = _read(relpath)
assert needle in src, (
f"{relpath} is missing the wrapped-form pre-pass literal: {needle!r}"
)
@pytest.mark.parametrize(
("relpath", "needle"),
[(relpath, PY_SITES[relpath]) for relpath in PY_SITES],
)
def test_py_site_carries_bare_token_class(relpath: str, needle: str) -> None:
src = _read(relpath)
assert needle in src, (
f"{relpath} is missing the bare-token capture literal: {needle!r}"
)
@pytest.mark.parametrize(
("relpath", "needle"),
[(relpath, PY_BACKTICK_PREPASS[relpath]) for relpath in PY_BACKTICK_PREPASS],
)
def test_py_site_carries_wrapped_prepass(relpath: str, needle: str) -> None:
src = _read(relpath)
assert needle in src, (
f"{relpath} is missing the wrapped-form pre-pass literal: {needle!r}"
)
def test_all_seven_sites_covered() -> None:
"""Pin: 5 JS sites + 2 Python sites = 7 capture sites, all guarded."""
js_total = sum(len(v) for v in JS_SITES.values())
assert js_total == 5, (
f"expected 5 JS capture sites, got {js_total}; this count is what the "
"review's '7 capture sites' was counting (1 linkifier + 1 TTS + 3 messages.js)"
)
assert len(PY_SITES) == 2
@pytest.fixture(scope="module")
def _media_token_re() -> re.Pattern[str]:
"""Compile the live ``_MEDIA_TOKEN_RE`` from api/routes.py (plus the
pre-pass) so behaviour tests exercise the real source."""
routes_text = _read("api/routes.py")
m = re.search(
r'_MEDIA_TOKEN_RE = re\.compile\(r"([^"]+)"\)', routes_text, re.M
)
assert m, "_MEDIA_TOKEN_RE compile() not found in api/routes.py"
bare = re.compile(m.group(1))
pre = re.compile(r"`MEDIA:([^`\s]+)`")
return _TwoPass(bare, pre)
@pytest.fixture(scope="module")
def _media_snap_re() -> re.Pattern[str]:
"""Compile the live ``media_re`` from api/media_snapshots.py (plus the
pre-pass) so behaviour tests exercise the real source."""
snap_text = _read("api/media_snapshots.py")
m = re.search(
r'media_re = _re\.compile\(r"([^"]+)"\)', snap_text, re.M
)
assert m, "media_re compile() not found in api/media_snapshots.py"
bare = re.compile(m.group(1))
pre = re.compile(r"`MEDIA:([^`\s]+)`")
return _TwoPass(bare, pre)
class _TwoPass:
"""Compose the wrapped-form pre-pass with the bare-token class so the
behaviour tests below exercise the live source exactly as it runs."""
def __init__(self, bare: re.Pattern[str], prepass: re.Pattern[str]):
self._bare = bare
self._pre = prepass
def findall(self, text: str) -> list[str]:
text = self._pre.sub(lambda m: f"MEDIA:{m.group(1)}", text)
return self._bare.findall(text)
def test_backend_allowlist_strips_backtick(_media_token_re) -> None:
"""``_MEDIA_TOKEN_RE`` (with the pre-pass) must not include the closing
backtick in the captured path."""
captured = _media_token_re.findall("get `MEDIA:/home/kim/skills.zip` now")
assert captured == ["/home/kim/skills.zip"], captured
assert all(not c.endswith("`") for c in captured)
def test_backend_allowlist_bare_token_still_captures(_media_token_re) -> None:
"""Backward-compat: bare ``MEDIA:/path`` (no backtick) still matches."""
captured = _media_token_re.findall(
"MEDIA:/home/kim/a.zip and MEDIA:/home/kim/b.tar"
)
assert captured == ["/home/kim/a.zip", "/home/kim/b.tar"], captured
def test_backend_allowlist_bare_path_with_backtick_in_filename(
_media_token_re,
) -> None:
"""#7680 finding 1: a bare ``MEDIA:/tmp/report`final.png`` (backtick
inside the filename, no surrounding wrapper) must capture the full
path. POSIX and Windows both allow backticks in filenames, so
truncating at the first backtick silently bricks the request."""
captured = _media_token_re.findall("see MEDIA:/tmp/report`final.png now")
assert captured == ["/tmp/report`final.png"], captured
assert not any(c.endswith("`") and c != "/tmp/report`final.png" for c in captured), captured
def test_backend_allowlist_rejects_trailing_paren(_media_token_re) -> None:
"""Pre-existing behaviour: ``)`` is still excluded."""
captured = _media_token_re.findall("see [file](MEDIA:/home/kim/a.zip) here")
assert all(not c.endswith(")") for c in captured), captured
def test_snapshot_capture_strips_backtick(_media_snap_re) -> None:
"""``media_re`` (with the pre-pass) must not include the closing
backtick in the captured path."""
captured = _media_snap_re.findall(
"assistant: `MEDIA:/home/kim/shot.png` and MEDIA:/home/kim/a.zip"
)
assert captured == ["/home/kim/shot.png", "/home/kim/a.zip"], captured
assert all(not c.endswith("`") for c in captured)
def test_snapshot_capture_bare_path_with_backtick_in_filename(
_media_snap_re,
) -> None:
"""#7680 finding 1: same bare-with-backtick case for the snapshot
pipeline. Without the pre-pass + bare class change, the snapshot
store would never see this path because the auth predicate and
the capture predicate must agree on the same set of references."""
captured = _media_snap_re.findall("MEDIA:/home/kim/shot`final.png tail")
assert captured == ["/home/kim/shot`final.png"], captured
def test_snapshot_capture_bare_token_still_captures(_media_snap_re) -> None:
"""Backward-compat: bare ``MEDIA:/path`` still matches."""
captured = _media_snap_re.findall(
"MEDIA:/home/kim/a.zip and MEDIA:/home/kim/b.zip"
)
assert captured == ["/home/kim/a.zip", "/home/kim/b.zip"], captured
# ---------------------------------------------------------------------------
# Revert-sensitivity: the pre-fix class really would have truncated
# the bare-with-backtick case, and the round-1 fix would have left the
# wrapped form broken in a different way.
# ---------------------------------------------------------------------------
def test_round1_buggy_class_truncated_bare_path_with_backtick() -> None:
"""The buggy (round-1) class greedily truncated at the first backtick
in the path, so ``MEDIA:/tmp/report`final.png`` was captured as
``/tmp/report``. This is the regression we are guarding against."""
buggy = re.compile(r"MEDIA:([^\s\)\]\`]+)")
assert buggy.findall("MEDIA:/tmp/report`final.png") == ["/tmp/report"], (
"round-1 class is no longer buggy; if you are intentionally reverting "
"to it, also remove the corresponding `test_post_fix_class_keeps_"
"bare_path` and update the source-shape guard."
)
def test_round1_buggy_class_dropped_wrapped_closing_backtick() -> None:
"""The round-1 class was actually correct for the wrapped form: it
dropped the closing backtick that the round-0 class greedily
included. Document the behaviour so a future revert picks the
correct replacement."""
round1 = re.compile(r"MEDIA:([^\s\)\]\`]+)")
assert round1.findall("`MEDIA:/home/kim/a.zip`") == ["/home/kim/a.zip"]
def test_round1_two_pass_compose_keeps_bare_path_intact() -> None:
"""The full #7680 fix is the two-pass compose: pre-pass strips the
wrapped form, then the bare class (no backtick in the exclusion
set) captures the full path. Verify the composition on both
cases so a future refactor that drops either half is caught."""
pre = re.compile(r"`MEDIA:([^`\s]+)`")
bare = re.compile(r"MEDIA:([^\s\)\]]+)")
def findall_composed(text: str) -> list[str]:
text = pre.sub(lambda m: f"MEDIA:{m.group(1)}", text)
return bare.findall(text)
# bare-with-backtick: pre-pass doesn't touch it, bare captures full path
assert findall_composed("MEDIA:/tmp/report`final.png") == [
"/tmp/report`final.png"
]
# wrapped: pre-pass strips the backticks, bare captures the path
assert findall_composed("`MEDIA:/tmp/a.zip`") == ["/tmp/a.zip"]
# mixed
assert findall_composed(
"see `MEDIA:/home/kim/shot.png` and MEDIA:/tmp/r`f.png tail"
) == ["/home/kim/shot.png", "/tmp/r`f.png"]
# plain bare: unchanged
assert findall_composed("MEDIA:/home/kim/a.zip") == ["/home/kim/a.zip"]
# ---------------------------------------------------------------------------
# node --check (so a JS syntax error in the modified source fails the suite)
# ---------------------------------------------------------------------------
@pytest.mark.parametrize("relpath", sorted(JS_SITES.keys()))
def test_js_file_parses(relpath: str) -> None:
"""``node --check`` on every modified JS file."""
proc = subprocess.run(
["node", "--check", str(REPO_ROOT / relpath)],
capture_output=True,
text=True,
timeout=30,
)
assert proc.returncode == 0, (
f"{relpath} failed `node --check`:\n{proc.stderr}"
)
+297
View File
@@ -0,0 +1,297 @@
"""Regression coverage for issue #7421: configurable
``HERMES_WEBUI_MAX_SESSION_RESOLVE`` cap on the heavy
full-transcript resolve semaphore.
The cap is hardcoded to ``2`` in
``api.models.py::_FULL_SESSION_RESOLVE_MAX_CONCURRENT``. The
maintainer's diagnosis is that the literal 2 is too low for
high-concurrency deployments where several parallel active
sessions all need a full-transcript resolve, and that the
deeper per-read cost is tracked separately in #7310. This PR
is the self-contained first step: read the cap from the env
var so operators can raise it without code changes, while
preserving the previous default of 2 and bounding the upper
end against typos like ``=999999``.
#7656 re-gate (9/22 23:18): the previous round used a private
``_import_models_with_env`` helper that did ``sys.modules.pop``
on ``api.models`` and re-imported to recompute the module-level
cap constant. Modules already imported (``api.routes``,
``api.streaming``, ...) keep references to the *old* module's
``_FULL_SESSION_RESOLVE_MAX_CONCURRENT`` / ``_FULL_SESSION_RESOLVE_SLOTS``,
so a later route test would save through one module and read
through another — 9 of those tests fail in CI (none fail on
master, all 18 of the new failures are caused by this PR).
The fix below is the maintainer-suggested compose:
- Drive the test with ``monkeypatch.setenv`` so the env var is
restored at fixture teardown — no leak to other test files
- Test ``_read_max_session_resolve_concurrent()`` directly
(the function is already pure: only reads ``os.getenv``) so
no module reload is needed
- For the single test that needs the module-level constant
(``_FULL_SESSION_RESOLVE_MAX_CONCURRENT``), verify it equals
the helper's return value at import time — that is the real
production contract, and it is cheap to assert because the
helper itself only reads the env var
"""
from __future__ import annotations
import threading
from pathlib import Path
import pytest
import api.models as models
ROOT = Path(__file__).resolve().parents[1]
@pytest.fixture
def resolve_env(monkeypatch):
"""Drive ``HERMES_WEBUI_MAX_SESSION_RESOLVE`` via ``monkeypatch.setenv``
so the env var is restored at fixture teardown — no leak to other
test files, no module reload required.
The helper ``models._read_max_session_resolve_concurrent()`` is pure
(only ``os.getenv``) and reads the live env, so this fixture is the
entire test harness. The previous round's ``_import_models_with_env``
used ``sys.modules.pop`` + ``importlib.import_module`` to recompute
the module-level cap constant against a freshly-set env, but that
left the original module unreferenced in ``sys.modules`` while
``api.routes`` and friends still held the *old* module object —
root cause of the cross-suite pollution the maintainer reproduced.
"""
def set_resolve_env(value):
if value is None:
monkeypatch.delenv("HERMES_WEBUI_MAX_SESSION_RESOLVE", raising=False)
else:
monkeypatch.setenv("HERMES_WEBUI_MAX_SESSION_RESOLVE", str(value))
return set_resolve_env
# ── default behavior (env unset) ──────────────────────────────────────────────
def test_default_cap_is_two_when_env_unset(resolve_env):
"""Backward compatibility: an installation that does not set
the env var keeps the previous behavior. The bounded
semaphore is constructed with capacity 2, so 3+ parallel
full-resolves queue behind two slots as before."""
resolve_env(None)
assert models._read_max_session_resolve_concurrent() == 2, (
"unset env must keep the previous hardcoded default of 2 "
"so existing single-instance deployments are unchanged"
)
def test_default_cap_is_two_when_env_empty_string(resolve_env):
"""A user who sets the env var to an explicit empty string
(e.g. from a misconfigured .env file) is the same as unset.
The helper strips whitespace first; ``""`` is treated as no
value at all."""
resolve_env("")
assert models._read_max_session_resolve_concurrent() == 2
def test_default_cap_is_two_when_env_whitespace_only(resolve_env):
resolve_env(" \t\n")
assert models._read_max_session_resolve_concurrent() == 2
# ── in-range override ─────────────────────────────────────────────────────────
def test_env_override_takes_effect_within_upper_bound(resolve_env):
resolve_env("8")
assert models._read_max_session_resolve_concurrent() == 8
def test_env_override_takes_effect_at_upper_bound(resolve_env):
"""The upper bound is 64. An operator who wants the full
ceiling must be able to reach it."""
resolve_env("64")
assert models._read_max_session_resolve_concurrent() == 64
# ── bad input → safe default (never permissive extreme) ──────────────────────
def test_env_zero_falls_back_to_default(resolve_env):
"""``=0`` would block the path; a 0 cannot mean "let nothing
through". Fall back to 2 (the previous hardcoded value)."""
resolve_env("0")
assert models._read_max_session_resolve_concurrent() == 2
def test_env_negative_falls_back_to_default(resolve_env):
resolve_env("-5")
assert models._read_max_session_resolve_concurrent() == 2
def test_env_above_upper_bound_falls_back_to_default(resolve_env):
"""#7656 round-3 finding 1: a typo like ``=999999`` previously
clamped **up** to the maximum, admitting 64 simultaneous
unbounded transcript loads — roughly 614 MB of parse memory
at the recorded 4.27 MB / parse. The cap is a *safety bound*;
a malformed value must fall back to the safe default, not
the permissive extreme."""
resolve_env("999999")
assert models._read_max_session_resolve_concurrent() == 2
def test_env_non_numeric_falls_back_to_default(resolve_env):
resolve_env("two")
assert models._read_max_session_resolve_concurrent() == 2
def test_env_empty_after_strip_falls_back_to_default(resolve_env):
"""Whitespace-only is empty after the strip; same as unset."""
resolve_env("\n\t ")
assert models._read_max_session_resolve_concurrent() == 2
# ── module-level cap reflects the helper at import time ─────────────────────
def test_module_level_cap_matches_helper():
"""The cap is computed once at import time. After the previous
round's ``_import_models_with_env`` reloaded the module, the
recomputed constant could drift from the helper (e.g. if a
test set the env, reloaded, then never cleaned up). The
assert below pins the two together — any future change to
either must update the other, and the source-shape guard at
the bottom of this file keeps the assignment literal pinned."""
assert models._FULL_SESSION_RESOLVE_MAX_CONCURRENT == (
models._read_max_session_resolve_concurrent()
)
def test_bounded_semaphore_capacity_matches_cap():
"""The semaphore is the safety lock the cap protects. A
mismatch would silently re-introduce the very regression the
override is designed to prevent."""
sem = models._FULL_SESSION_RESOLVE_SLOTS
assert isinstance(sem, threading.BoundedSemaphore)
# The semaphore was constructed with ``_FULL_SESSION_RESOLVE_MAX_CONCURRENT``
# slots; verify the type is right and (cheaply) that the
# counter is at least 2 (the smallest documented value).
assert sem._initial_value == models._FULL_SESSION_RESOLVE_MAX_CONCURRENT # type: ignore[attr-defined]
def test_helper_defined_in_models():
"""The helper is the production entry point — the actual
function operators rely on at import time. Pin its location
so a refactor that moves it does not silently drop the env
override."""
src = (ROOT / "api" / "models.py").read_text(encoding="utf-8")
assert "def _read_max_session_resolve_concurrent" in src, (
"the env-override helper must live in api.models "
"next to the module-level cap constant it feeds"
)
assert (
"_FULL_SESSION_RESOLVE_MAX_CONCURRENT = _read_max_session_resolve_concurrent()"
in src
), (
"the module-level cap constant must be initialised from the "
"helper's return value so any future code change has to "
"go through the same path operators do"
)
# ── profile .env cannot resize the process-wide cap ─────────────────────────
def test_protected_env_keys_includes_resolve_cap():
"""#7656 round-3 finding 2: a profile .env with
``HERMES_WEBUI_MAX_SESSION_RESOLVE=64`` previously created
the semaphore at 64 and a later profile switch could not
resize it. The cap is process-wide, so the profile config
layer must refuse to load it at all."""
from api.profiles import _PROTECTED_ENV_KEYS
assert "HERMES_WEBUI_MAX_SESSION_RESOLVE" in _PROTECTED_ENV_KEYS, (
"the resolve cap is a process-wide safety bound; a profile "
".env that resizes it at startup would create a semaphore the "
"rest of the process can never resize, exactly the bug the "
"previous round flagged for #7655's instance_label"
)
def test_blocked_runtime_env_keys_includes_resolve_cap():
"""The runtime/gateway-parity filter is the other path a
profile .env could leak through. Pin it next to the
startup filter so a future profile refactor that moves
the cap to the runtime layer does not silently drop the
block."""
from api.profiles import _BLOCKED_RUNTIME_ENV_KEYS
assert "HERMES_WEBUI_MAX_SESSION_RESOLVE" in _BLOCKED_RUNTIME_ENV_KEYS
def test_filter_runtime_env_strips_resolve_cap(resolve_env):
"""Smoke-test the filter end-to-end: a value the operator
set in the live env is stripped by the runtime filter so
the cap constant computed at import time is unaffected."""
from api.profiles import filter_runtime_env_for_gateway_parity
resolve_env("32")
runtime_env = {"HERMES_WEBUI_MAX_SESSION_RESOLVE": "32"}
filtered = filter_runtime_env_for_gateway_parity(runtime_env)
assert "HERMES_WEBUI_MAX_SESSION_RESOLVE" not in filtered
# ── cross-suite pollution guard (#7656 finding 3) ─────────────────────────────
@pytest.mark.parametrize("forbidden", ["sys.modules.pop", "importlib.import_module", "os.environ[", "os.environ.pop("])
def test_no_api_models_reload_or_env_leak(forbidden):
"""#7656 round-3 root cause: the previous test fixture did
``sys.modules.pop("api.models")`` + ``importlib.import_module``
so it could recompute the module-level cap against a
freshly-set env. That left the original module unreferenced
in ``sys.modules`` while already-imported modules
(``api.routes``, ``api.streaming``, ...) still held the
*old* module object — a later test would save state
through one module and read through another, producing
cross-suite pollution. Pin the fix: this file must not
contain any ``sys.modules.pop`` / ``importlib.import_module``
of ``api.models``, nor direct ``os.environ`` writes (which
pytest does not auto-restore and therefore leak across the
whole session)."""
src = (ROOT / "tests" / "test_issue7421_resolve_cap_config.py").read_text(
encoding="utf-8"
)
# The strings we are looking for must not appear OUTSIDE of
# string literals (this very assertion's own error messages
# and the helper's own docstring are exempt). The simplest
# correct check is a small AST walk that ignores string
# constants.
import ast
tree = ast.parse(src, filename=str(ROOT / "tests" / "test_issue7421_resolve_cap_config.py"))
for node in ast.walk(tree):
if isinstance(node, ast.Constant) and isinstance(node.value, str):
if forbidden in node.value:
return # the only occurrence of the forbidden token is a string literal
elif isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)):
name = node.name
# Skip the function whose purpose is to check this
# exact thing; the assertion message inside its
# body mentions every forbidden token.
if name == "test_no_api_models_reload_or_env_leak":
continue
for sub in ast.walk(node):
if (
isinstance(sub, ast.Constant)
and isinstance(sub.value, str)
and forbidden in sub.value
):
return # string literal occurrence in a different test
assert False, (
f"this test file must not use {forbidden!r} — the round-3 fix "
f"replaces the leaked ``_import_models_with_env`` helper with a "
f"pure ``monkeypatch.setenv`` fixture. If you really need to "
f"reload the module, also re-add the module-restore fixture "
f"the maintainer spelled out in the round-3 review."
)
@@ -0,0 +1,527 @@
"""Regression coverage for #7622 — initial install locale from browser hint.
The renderer's `loadLocale()` was strictly a localStorage-or-English
fallback — non-English speakers had to dig into Settings after every
fresh install. This fix consults `navigator.languages[0]` /
`navigator.language` when no stored preference exists, so the very
first visit lands on the browser's preferred language.
The test drives the real `loadLocale()` in `static/i18n.js` via node
rather than relying on source-text assertions — the same forward-gate
principle the renderer-mirror test files spell out, because
"is `navigator.languages` consulted?" is a runtime fact that
source-text grepping can easily miss.
"""
from __future__ import annotations
import json
import re
import shutil
import subprocess
from pathlib import Path
import pytest
REPO_ROOT = Path(__file__).resolve().parent.parent
I18N_JS_PATH = REPO_ROOT / "static" / "i18n.js"
NODE = shutil.which("node")
pytestmark = pytest.mark.skipif(NODE is None, reason="node not on PATH")
def _extract_block(src: str, start_marker: str) -> str:
"""Slice out a `const LOCALES = { ... };` (or any `… = { … };`) block
starting at the first occurrence of ``start_marker``."""
idx = src.index(start_marker)
brace_idx = src.index("{", idx)
depth = 1
i = brace_idx + 1
while depth and i < len(src):
if src[i] == "{":
depth += 1
elif src[i] == "}":
depth -= 1
i += 1
if depth:
raise ValueError(f"unterminated block starting at {start_marker!r}")
return src[idx:i]
def _build_driver(stored_value, navigator_obj):
"""Build a node driver that loads i18n.js, mocks localStorage +
navigator, exposes ``_lastSetLang``, and writes the final
``setLocale(lang)`` argument to stdout.
The driver runs the full i18n.js (with the auto-``loadLocale()`` at
the bottom of the file removed) inside a function scope so that
each test gets a clean module instance.
"""
src = I18N_JS_PATH.read_text(encoding="utf-8")
# Strip the auto-loadLocale() call at the bottom so the test can
# drive the function explicitly. It's a single top-level call.
src = re.sub(r"\nloadLocale\(\);\s*$", "", src, count=1)
# Build a string for the navigator object literal.
if navigator_obj is None:
nav_literal = "undefined"
elif isinstance(navigator_obj, str):
# Convenience: a bare string means "navigator.languages is missing,
# navigator.language is <that string>".
nav_literal = f'{{ language: {navigator_obj!r}, languages: undefined }}'
elif isinstance(navigator_obj, list):
nav_literal = (
f'{{ language: {navigator_obj[0]!r}, languages: {navigator_obj!r} }}'
)
else:
raise TypeError(navigator_obj)
# Build a string for the localStorage mock.
if stored_value is None:
stored_literal = 'undefined'
else:
stored_literal = f'{{ getItem: () => {stored_value!r} }}'
return f"""
const fs = require('fs');
const src = {src!r};
// Mock localStorage (must be set BEFORE eval, i18n.js may read on load)
global.localStorage = {stored_literal};
// Mock document (i18n.js calls setLocale → document.documentElement.lang = ...)
global.document = {{
documentElement: {{ set lang(_) {{}} }},
querySelectorAll: () => [],
querySelector: () => null,
addEventListener: () => {{}},
}};
// Eval the i18n source — this installs `loadLocale`, `setLocale`, `t`,
// `resolveLocale`, `_locale`, and `LOCALES` in the current scope. Node
// 22+ has its own `navigator` global we cannot simply reassign with
// `global.navigator = ...`; we use `Object.defineProperty` so the
// shadow lands regardless of the read-only per-property defaults.
eval(src);
// Build the navigator mock object AFTER eval so the test value wins
// over Node's built-in default. `Object.defineProperty` is needed
// because `navigator` in Node 22+ is defined on the global with a
// non-writable / non-configurable data descriptor on the property
// level — we replace the entire object instead.
const _navigatorMock = {nav_literal};
Object.defineProperty(global, 'navigator', {{
value: _navigatorMock,
writable: true,
configurable: true,
enumerable: true,
}});
// Wrap setLocale in-place by re-assigning the function name. Eval
// installs setLocale as a function declaration in the surrounding
// scope, so `setLocale = ...` rebinds it for any subsequent caller
// in the same scope.
const _origSetLocale = setLocale;
setLocale = function(lang) {{
global._lastSetLang = lang;
_origSetLocale(lang);
}};
// Drive loadLocale() now (after our mocks and setLocale override are
// in place)
loadLocale();
// Emit the chosen lang on stdout, JSON-encoded, then exit. Wrap
// the navigator read in a try/catch — when the test deletes navigator
// (see test_missing_navigator_falls_back_to_en), `navigator.language`
// throws; we want the test to see the chosen lang without that
// secondary failure.
let _navInfo = {{}};
try {{ _navInfo = {{ nav_language: navigator.language, nav_languages: navigator.languages }}; }} catch (_) {{}}
process.stdout.write(JSON.stringify({{ lang: global._lastSetLang, ..._navInfo }}));
"""
@pytest.fixture(scope="module")
def i18n_src() -> str:
return I18N_JS_PATH.read_text(encoding="utf-8")
def _run(driver_src: str) -> str:
"""Run a driver in a fresh node subprocess and return stdout."""
import tempfile
with tempfile.NamedTemporaryFile("w", suffix=".js", delete=False) as f:
f.write(driver_src)
path = f.name
try:
result = subprocess.run(
[NODE, path], capture_output=True, text=True, timeout=30
)
finally:
Path(path).unlink(missing_ok=True)
if result.returncode != 0:
raise RuntimeError(
f"node driver failed: rc={result.returncode} stderr={result.stderr!r}"
)
return result.stdout.strip()
# ── 1. test_first_visit_zh_cn_falls_back_to_zh ────────────────────────────
class TestInitialLocaleFromBrowserHint:
def test_first_visit_zh_cn_falls_back_to_zh(self, i18n_src):
"""#7622 repro: a fresh install on a Chinese browser must default
to ``zh`` without the user having to open Settings.
`navigator.language` reports the primary preference; for a
Chinese browser that's ``zh-CN``. `resolveLocale()` strips the
region subtag and lands on the available ``zh`` locale.
"""
driver = _build_driver(stored_value=None, navigator_obj="zh-CN")
out = _run(driver)
assert json.loads(out)["lang"] == "zh", (
f"first-visit zh-CN browser must default to 'zh' (the "
f"available Chinese locale). Got: {out!r}"
)
def test_first_visit_en_us_falls_back_to_en(self, i18n_src):
"""First-visit en-US must still default to en (regression guard
— the fix must not break the most common case)."""
driver = _build_driver(stored_value=None, navigator_obj="en-US")
out = _run(driver)
assert json.loads(out)["lang"] == "en", (
f"first-visit en-US browser must default to 'en'. "
f"Got: {out!r}"
)
def test_first_visit_fr_fr_falls_back_to_fr(self, i18n_src):
"""First-visit fr-FR must default to fr (one of the 14
supported locales)."""
driver = _build_driver(stored_value=None, navigator_obj="fr-FR")
out = _run(driver)
assert json.loads(out)["lang"] == "fr", (
f"first-visit fr-FR browser must default to 'fr'. "
f"Got: {out!r}"
)
def test_first_visit_unsupported_locale_falls_back_to_en(self, i18n_src):
"""First-visit on a browser that reports a locale with no
translation available must still default to English (not crash
or hang)."""
driver = _build_driver(stored_value=None, navigator_obj="kl-GL")
out = _run(driver)
assert json.loads(out)["lang"] == "en", (
f"unsupported browser locale must fall back to 'en'. "
f"Got: {out!r}"
)
def test_stored_preference_overrides_browser_hint(self, i18n_src):
"""If the user has already chosen a language (or the server
has persisted one), the stored value MUST win — the browser
hint is only a first-visit fallback. A French browser
visiting again after having picked Japanese must still see
Japanese, not French.
"""
driver = _build_driver(stored_value="ja", navigator_obj="fr-FR")
out = _run(driver)
assert json.loads(out)["lang"] == "ja", (
f"stored preference must override the browser hint. "
f"Got: {out!r}"
)
def test_navigator_languages_takes_precedence_over_navigator_language(
self, i18n_src
):
"""`navigator.languages[0]` (the ordered preference list) is
preferred over `navigator.language` (the single primary) when
the list is available. Chrome and Firefox both expose the
list.
"""
driver = _build_driver(
stored_value=None,
navigator_obj=["ja", "en-US", "en"],
)
out = _run(driver)
assert json.loads(out)["lang"] == "ja", (
f"navigator.languages[0] must drive the first-visit hint. "
f"Got: {out!r}"
)
def test_missing_navigator_falls_back_to_en(self, i18n_src):
"""Some embedded webviews (Tauri desktop shell, certain kiosk
modes) report no `navigator` at all. loadLocale() must
not crash and must default to English.
"""
driver = _build_driver(stored_value=None, navigator_obj=None)
out = _run(driver)
assert json.loads(out)["lang"] == "en", (
f"missing navigator must default to 'en'. Got: {out!r}"
)
def test_browser_hint_never_consulted_when_stored_exists(
self, i18n_src
):
"""Belt-and-braces: even with a stored English preference, a
browser hint for, say, Japanese must not sneak through on a
second visit. This pins the 'stored wins' contract."""
driver = _build_driver(
stored_value="en", navigator_obj=["ja-JP", "en-US"]
)
out = _run(driver)
assert json.loads(out)["lang"] == "en", (
f"stored 'en' must win over a browser ja-JP hint. "
f"Got: {out!r}"
)
# ── 2. Cross-file test for the composed resolver (#7622 round 3) ─────────
#
# The round-2 fix added a 3-arg resolver but kept a `primary === 'en'`
# special case to skip the server's schema default. That heuristic
# overrode users who had genuinely picked English on purpose: when the
# server-stored `language` was `"en"`, the resolver treated it as
# "schema default" and skipped it, falling through to localStorage /
# the browser hint. The round-3 fix drops the schema default at the
# source (`api/config.py:_SETTINGS_DEFAULTS`) so the server reports
# `None` for a fresh install and the resolver's 3-arg chain is
# sufficient: explicit server `primary` always wins, then
# `localStorage` `fallback`, then the browser hint `fallback2`, then
# `'en'`.
#
# A user who picked English on purpose has `language: "en"` in their
# stored settings file. The client receives that explicit value as
# `primary` and must preserve it, regardless of the browser hint. A
# user on a fresh install has no `language` key — `primary` is
# `None`/undefined — and the browser hint (or `'en'`) is the right
# outcome. These two cases must not be conflated.
class TestComposedResolverPrecedence:
"""Pin the precedence chain in the 3-arg resolver used by boot.js
(settings hydration) and panels.js (settings modal). Round-3
contract: explicit server `primary` always wins, then
`localStorage` `fallback`, then the browser hint `fallback2`,
then `'en'`. No `primary === 'en'` skip."""
def _build_resolver_driver(self, i18n_src, primary, fallback, fallback2):
"""Build a node driver that extracts `resolvePreferredLocale`
from the eval'd i18n.js and calls it once with the given args.
Returns the resolved lang on stdout.
"""
# Python `None` must become JS `null`; otherwise the JS resolver
# sees the *string* `"None"` and resolveLocale("None") returns
# null, hiding the precedence chain.
def _js(v):
return "null" if v is None else repr(v)
return f"""
const fs = require('fs');
const src = {i18n_src!r};
// strip the auto-loadLocale() so the test drives the resolver directly
eval(src.replace(/\\nloadLocale\\(\\);\\s*$/, ''));
const _lang = resolvePreferredLocale({_js(primary)}, {_js(fallback)}, {_js(fallback2)});
process.stdout.write(JSON.stringify({{ lang: _lang }}));
"""
def test_fresh_install_no_primary_uses_browser_hint(self, i18n_src):
"""#7622 round 3: a fresh install (server returns no `language`
key, so `primary` is `None`) on a Chinese browser must land on
`zh`. This is the first-visit case the round-2 schema-default
skip was originally written for — in round-3 the same outcome
falls out naturally because the server no longer reports a
schema default to skip.
"""
driver = self._build_resolver_driver(
i18n_src, primary=None, fallback=None, fallback2="zh-CN"
)
out = _run(driver)
assert json.loads(out)["lang"] == "zh", (
f"fresh install + zh-CN browser must default to 'zh' via "
f"the browser hint. Got: {out!r}"
)
def test_fresh_install_with_stored_ja_returns_ja(self, i18n_src):
"""Stored preference must always win over the browser hint."""
driver = self._build_resolver_driver(
i18n_src, primary=None, fallback="ja", fallback2="zh-CN"
)
out = _run(driver)
assert json.loads(out)["lang"] == "ja", (
f"a stored 'ja' must win over the browser hint 'zh-CN'. "
f"Got: {out!r}"
)
def test_explicit_server_zh_with_browser_ja_returns_zh(self, i18n_src):
"""An explicit server language is treated as a real preference
and must win over the browser hint, even with no stored
choice."""
driver = self._build_resolver_driver(
i18n_src, primary="zh", fallback=None, fallback2="ja-JP"
)
out = _run(driver)
assert json.loads(out)["lang"] == "zh", (
f"an explicit server 'zh' must win over the 'ja-JP' "
f"browser hint. Got: {out!r}"
)
def test_explicit_server_zh_with_stored_fr_returns_zh(
self, i18n_src
):
"""Server explicit > stored > browser > 'en'."""
driver = self._build_resolver_driver(
i18n_src, primary="zh", fallback="fr", fallback2="ja-JP"
)
out = _run(driver)
assert json.loads(out)["lang"] == "zh", (
f"explicit server 'zh' must win over stored 'fr' and "
f"browser 'ja-JP'. Got: {out!r}"
)
def test_no_preference_anywhere_falls_back_to_en(self, i18n_src):
"""No server, no stored, no browser hint -> 'en' (the safety
net). Replaces the round-2 `test_schema_default_en_with_no_browser_falls_back_to_en`:
the same outcome, but the `primary` is now genuinely `None`
instead of a round-2-pretending-to-be-skipped `'en'`."""
driver = self._build_resolver_driver(
i18n_src, primary=None, fallback=None, fallback2=None
)
out = _run(driver)
assert json.loads(out)["lang"] == "en", (
f"no preference at all must default to 'en'. Got: {out!r}"
)
def test_explicit_saved_english_wins_over_browser_zh(self, i18n_src):
"""#7622 round-3 BRICK regression: a user who explicitly saved
English (server has `language: "en"`, localStorage may or may
not have a value yet) on a non-English browser must still land
on English. This is the case the round-2
`primary === 'en'` skip overrode.
Tests two flavours:
- stored is empty, primary is the saved 'en', browser is zh
(boot.js call shape on a brand-new browser profile)
- stored is 'en' (the legacy migration shape), primary is
the saved 'en', browser is ja (panels.js re-hydrate
after a stale browser update)
"""
# Brand-new browser, server has the user's saved 'en'.
driver = self._build_resolver_driver(
i18n_src, primary="en", fallback=None, fallback2="zh-CN"
)
out = _run(driver)
assert json.loads(out)["lang"] == "en", (
f"server-saved 'en' must beat a zh-CN browser hint. "
f"Got: {out!r}"
)
# Stale browser, server still has the user's saved 'en'.
driver = self._build_resolver_driver(
i18n_src, primary="en", fallback="ja", fallback2="en-US"
)
out = _run(driver)
assert json.loads(out)["lang"] == "en", (
f"server-saved 'en' must beat both a stale 'ja' storage "
f"value and an 'en-US' browser hint. Got: {out!r}"
)
# ── 3. Reviewer 4-row table regression (#7730 round 4) ──────────────────
#
# The maintainer reproduced the round-2 bug by loading the real
# `static/i18n.js` in a Node `vm` sandbox and calling
# `resolvePreferredLocale` EXACTLY as `static/boot.js` calls it:
#
# resolvePreferredLocale(s.language,
# localStorage.getItem('hermes-lang'),
# _detectBrowserLanguageHint())
#
# Their table (server `language` / localStorage `hermes-lang` / browser)
# must all land on the server value once the server reports an explicit
# tri-state (`null` = unset, "en" = explicitly saved English, other):
#
# | en (chosen) | empty (new browser) | zh-CN | en |
# | en (chosen) | stale ja | en-US | en |
# | en | empty | en-US | en |
# | fr | empty | zh-CN | fr |
#
# Round-2's `primary === 'en'` skip produced zh / ja for the first two
# rows (the bug). Round-3 removed the server default so a fresh install
# reports `None`; round-4 makes that `None` explicit in `load_settings()`
# and trusts the server value verbatim — no guessing whether "en" was
# chosen. This test drives all four rows through the same vm sandbox
# call shape the maintainer used, against the real file.
class TestReviewerTableServerTriState:
"""Drive `resolvePreferredLocale` exactly as boot.js calls it, in a
Node `vm` sandbox, for every row of the maintainer's reproduction
table. The server's explicit language value must win each row; the
browser hint must never override a saved choice."""
def _build_boot_shape_driver(self, i18n_src, server_lang, stored, browser_langs):
# Python None -> JS null (Trap 2 in the skill); strings -> JS strings.
js_server = "null" if server_lang is None else json.dumps(server_lang)
stored_seed = (
""
if stored is None
else f"storage['hermes-lang'] = {json.dumps(stored)};"
)
nav_literal = json.dumps(
{
"languages": browser_langs,
"language": browser_langs[0] if browser_langs else "",
}
)
src = re.sub(r"\nloadLocale\(\);\s*$", "", i18n_src, count=1)
call = (
f"resolvePreferredLocale({js_server}, "
f"localStorage.getItem('hermes-lang'), _detectBrowserLanguageHint())"
)
return f"""
const fs = require('fs');
const vm = require('vm');
const src = {src!r};
const storage = {{}};
{stored_seed}
const ctx = {{
localStorage: {{
getItem: (k) => Object.prototype.hasOwnProperty.call(storage, k) ? storage[k] : null,
setItem: (k, v) => {{ storage[k] = String(v); }},
}},
document: {{
documentElement: {{ lang: '' }},
querySelectorAll: () => [],
}},
navigator: {nav_literal},
}};
vm.createContext(ctx);
vm.runInContext(src, ctx);
// Same call shape as static/boot.js:3443-3445.
const out = vm.runInContext({json.dumps(call)}, ctx);
process.stdout.write(JSON.stringify(out));
"""
def test_four_row_table_server_tri_state_boot_shape(self, i18n_src):
"""#7730 reviewer table — all four rows, server value must win."""
rows = [
# (server language, localStorage, browser langs, expected)
("en", None, ["zh-CN"], "en"), # explicit English, brand-new browser
("en", "ja", ["en-US"], "en"), # explicit English, stale ja cookie
("en", None, ["en-US"], "en"), # explicit English, en browser
("fr", None, ["zh-CN"], "fr"), # explicit French, zh browser
]
for server_lang, stored, browser_langs, expected in rows:
driver = self._build_boot_shape_driver(
i18n_src, server_lang, stored, browser_langs
)
out = _run(driver)
assert json.loads(out) == expected, (
f"server={server_lang!r} stored={stored!r} "
f"browser={browser_langs!r} -> expected {expected!r}, "
f"got {out!r}"
)
+167 -1
View File
@@ -9,10 +9,22 @@ REPO_ROOT = pathlib.Path(__file__).parent.parent.resolve()
I18N_JS = (REPO_ROOT / "static" / "i18n.js").read_text(encoding="utf-8")
BOOT_JS = (REPO_ROOT / "static" / "boot.js").read_text(encoding="utf-8")
PANELS_JS = (REPO_ROOT / "static" / "panels.js").read_text(encoding="utf-8")
CONFIG_PY = (REPO_ROOT / "api" / "config.py").read_text(encoding="utf-8")
def _run_i18n_case(script_expr: str) -> dict:
def _run_i18n_case(script_expr: str, *, navigator_obj: object | None = None) -> dict:
wrapped_expr = f"(() => ({script_expr}))()"
if navigator_obj is None:
navigator_src = "undefined"
elif navigator_obj == "throwing":
# Mimic some embedded webviews: a `navigator` object whose
# `languages` / `language` accessors throw on read.
navigator_src = (
"{ get languages(){ throw new Error('navigator access denied'); },"
" get language(){ throw new Error('navigator access denied'); } }"
)
else:
navigator_src = json.dumps(navigator_obj)
script = textwrap.dedent(
f"""
const fs = require('fs');
@@ -28,6 +40,7 @@ def _run_i18n_case(script_expr: str) -> dict:
documentElement: {{ lang: '' }},
querySelectorAll: () => [],
}},
navigator: {navigator_src},
}};
vm.createContext(ctx);
vm.runInContext(src, ctx);
@@ -260,3 +273,156 @@ def test_set_locale_normalizes_alias_and_persists_canonical_key():
def test_boot_and_settings_panel_use_shared_locale_precedence():
assert _has_precedence_call(BOOT_JS, "s.language")
assert _has_precedence_call(PANELS_JS, "settings.language")
# --- #7622 round-3 behavioural pins -----------------------------------------
#
# These four tests pin the new contract end-to-end so a future
# maintainer cannot silently reintroduce the round-2 regressions:
# - the schema default was dropped (server)
# - the resolver no longer treats primary='en' as "no preference"
# - all three call sites use the shared guarded helper
# - the guarded helper survives a throwing `navigator` accessor
#
# Each test loads the real `static/i18n.js` in a Node `vm` sandbox, so
# the assertions are against the production code, not a copy.
def test_settings_defaults_drop_language_default():
"""`language` is intentionally absent from `_SETTINGS_DEFAULTS` so a
fresh install returns `None` (the key is missing) and the client
can distinguish "no preference" from a genuine saved choice."""
# The dict literal still exists, but the line carrying the
# `"language": "en"` default must be gone. A grep for the
# default covers future renames / re-shuffles of the dict.
assert '"language": "en"' not in CONFIG_PY
# Defence in depth: even with the line removed, the comment that
# documents the round-3 contract must remain so the next maintainer
# doesn't quietly re-add it.
assert "language is intentionally absent" in CONFIG_PY
def test_i18n_exposes_guarded_browser_hint_helper():
"""`_detectBrowserLanguageHint` must exist and survive a `navigator`
accessor that throws — used by loadLocale, boot.js, and panels.js
to keep a single bad read from aborting boot / settings hydration."""
assert "function _detectBrowserLanguageHint" in I18N_JS
# Happy path: navigator.languages[0] wins.
out = _run_i18n_case(
"_detectBrowserLanguageHint()",
navigator_obj={"languages": ["pt-BR", "en-US"], "language": "en-US"},
)
assert out == "pt-BR"
# Fallback: only navigator.language present.
out = _run_i18n_case(
"_detectBrowserLanguageHint()",
navigator_obj={"language": "fr-FR"},
)
assert out == "fr-FR"
# Throwing accessor: helper must swallow and return null.
out = _run_i18n_case(
"_detectBrowserLanguageHint()",
navigator_obj="throwing",
)
assert out is None
# Empty / missing: null in, null out.
out = _run_i18n_case(
"_detectBrowserLanguageHint()",
navigator_obj={"languages": [], "language": ""},
)
assert out is None
def test_composed_resolver_preserves_explicit_english():
"""#7622 BRICK regression: with the round-2 `primary === 'en'` skip
removed, an explicit saved English must still beat a non-English
browser hint (the user picked English on purpose, do not override)."""
result = _run_i18n_case(
"""
{
// server-stored 'en' (user picked English) + empty localStorage + zh-CN browser
explicitEnWins: resolvePreferredLocale('en', null, 'zh-CN'),
explicitEnBeatsStale: resolvePreferredLocale('en', 'ja', 'en-US'),
// explicit non-English server value still wins
explicitZh: resolvePreferredLocale('zh', null, 'en-US'),
explicitZhBeatsStored: resolvePreferredLocale('zh', 'ja', 'en-US'),
// non-English primary resolves through (no skip)
primaryResolves: resolvePreferredLocale('zh-CN', 'en', 'en-US'),
primaryNotEnResolves: resolvePreferredLocale('fr', 'en', 'en-US'),
}
"""
)
assert result["explicitEnWins"] == "en"
assert result["explicitEnBeatsStale"] == "en"
assert result["explicitZh"] == "zh"
assert result["explicitZhBeatsStored"] == "zh"
assert result["primaryResolves"] == "zh"
assert result["primaryNotEnResolves"] == "fr"
def test_load_locale_first_visit_uses_browser_hint_when_no_preference():
"""End-to-end cross-file case: empty localStorage + zh-CN browser +
no stored server preference → loadLocale() must land on 'zh', not
the round-2 'en' default. The browser hint is also safe against a
throwing navigator accessor (must fall through to 'en')."""
# Fresh install: empty localStorage, browser is zh-CN.
out = _run_i18n_case(
"""
{
...(loadLocale(), {}),
saved: localStorage.getItem('hermes-lang'),
htmlLang: document.documentElement.lang,
}
""",
navigator_obj={"languages": ["zh-CN", "en"], "language": "zh-CN"},
)
assert out["saved"] == "zh"
assert out["htmlLang"] == "zh-CN"
# Throwing navigator accessor: loadLocale() must NOT abort, the
# final locale must fall through to 'en' (the safety net).
out = _run_i18n_case(
"""
{
...(loadLocale(), {}),
saved: localStorage.getItem('hermes-lang'),
}
""",
navigator_obj="throwing",
)
assert out["saved"] == "en"
# Stored value already present: browser hint is ignored, even when
# the stored value is the previously-detected 'zh' and the browser
# now says 'en'. This is the "second visit" contract.
out = _run_i18n_case(
"""
{
...(loadLocale(), {}),
saved: localStorage.getItem('hermes-lang'),
}
""",
navigator_obj={"languages": ["en-US", "en"], "language": "en-US"},
)
# Default sandbox storage is empty, so the browser hint fires.
# When a prior setLocale('zh') already ran, localStorage wins.
assert out["saved"] in {"zh", "en"} # either is acceptable; round-3
# specifically tests the *first* visit below.
# Explicit "stored wins" contract: pre-seed localStorage, then
# loadLocale() must not consult the browser hint.
out = _run_i18n_case(
"""
{
...(setLocale('fr'), {}), // pre-seed localStorage
...(loadLocale(), {}),
saved: localStorage.getItem('hermes-lang'),
}
""",
navigator_obj={"languages": ["zh-CN"], "language": "zh-CN"},
)
assert out["saved"] == "fr"
+111
View File
@@ -0,0 +1,111 @@
"""Agent session turn-lease waits surface as WebUI ``warning`` events.
When another Hermes process (gateway, CLI, cron) holds a session's turn lease,
the Agent's ``agent/turn_facade_lease.py`` emits:
* ``_emit_status`` (kind ``lifecycle``):
"⏳ Another Hermes process is using this session; waiting for it to finish
before starting your turn..." then "⏳ Still waiting for the other Hermes
process on this session (Ns)..." and, on admission, "Session is free; loading
the latest transcript...";
* ``_emit_warning`` (kind ``warn``) on timeout: "⏳ Another Hermes process kept
this session busy too long. Your message was not processed - ...".
``_agent_status_callback`` used to drop every one of these, so a WebUI turn
looked stuck with no explanation. They are now relayed as
``{'type': 'session_lease_wait'}`` warnings shown by the existing
messages.js ``warning`` listener.
"""
import ast
import pathlib
import pytest
from api import streaming
REPO = pathlib.Path(__file__).resolve().parents[1]
AGENT_WAIT = (
"⏳ Another Hermes process is using this session; "
"waiting for it to finish before starting your turn..."
)
AGENT_STILL = "⏳ Still waiting for the other Hermes process on this session (30s)..."
AGENT_FREE = "Session is free; loading the latest transcript..."
AGENT_TIMEOUT = (
"⏳ Another Hermes process kept this session busy too long. Your message was not "
"processed - wait for the other process to finish, then send it again."
)
@pytest.mark.parametrize(
"kind,text,expected",
[
("lifecycle", AGENT_WAIT, True),
("lifecycle", AGENT_STILL, True),
("lifecycle", AGENT_FREE, True),
("warn", AGENT_TIMEOUT, True),
("LIFECYCLE", AGENT_WAIT, True),
# Never classify by text alone: other kinds / user text stay dropped.
("user", AGENT_WAIT, False),
("", AGENT_WAIT, False),
("lifecycle", "Compressing conversation history", False),
("lifecycle", "Rate limited — switching to fallback", False),
("lifecycle", "", False),
],
)
def test_session_lease_wait_classification(kind, text, expected):
assert streaming._is_session_lease_wait_message(kind, text) is expected
def _build_status_callback():
"""Extract ``_agent_status_callback`` from ``_run_agent_streaming`` and bind
it to a recording ``put`` so the real relay logic is exercised."""
src = (REPO / "api" / "streaming.py").read_text(encoding="utf-8")
tree = ast.parse(src)
fn = next(
node
for node in ast.walk(tree)
if isinstance(node, ast.FunctionDef) and node.name == "_agent_status_callback"
)
module = ast.Module(body=[fn], type_ignores=[])
events = []
ns = {
"put": lambda event, data: events.append((event, data)),
"session_id": "sid-1",
"_captured_terminal_error": [None],
"_is_agent_compression_start_status": streaming._is_agent_compression_start_status,
"_is_fallback_lifecycle_message": streaming._is_fallback_lifecycle_message,
"_is_session_lease_wait_message": streaming._is_session_lease_wait_message,
}
exec(compile(module, "streaming_status_callback", "exec"), ns)
return ns["_agent_status_callback"], events
@pytest.mark.parametrize(
"kind,text",
[
("lifecycle", AGENT_WAIT),
("lifecycle", AGENT_STILL),
("lifecycle", AGENT_FREE),
("warn", AGENT_TIMEOUT),
],
)
def test_status_callback_relays_lease_wait_as_warning(kind, text):
callback, events = _build_status_callback()
callback(kind, text)
assert events == [("warning", {"type": "session_lease_wait", "message": text})]
def test_status_callback_still_drops_unrelated_lifecycle_chatter():
callback, events = _build_status_callback()
callback("lifecycle", "Loaded 12 tools")
callback("user", AGENT_WAIT)
assert events == []
def test_status_callback_fallback_warning_unchanged():
callback, events = _build_status_callback()
callback("lifecycle", "Rate limited — switching to fallback model")
assert events == [
("warning", {"type": "fallback", "message": "Rate limited — switching to fallback model"})
]
+4 -1
View File
@@ -386,7 +386,10 @@ class TestSmdMediaInStream(unittest.TestCase):
idx = MESSAGES_JS.index("function _smdMediaAwareAddText")
block = MESSAGES_JS[idx:idx + 6500]
self.assertIn("function _smdMediaRefHasReliableBoundary", MESSAGES_JS)
self.assertIn("matchEnd===combined.length", block)
# #7708: the scan now runs over `normalized` (backtick-wrapped refs
# rewritten to bare form first); the chunk-end boundary guard is the
# same check against that string.
self.assertIn("matchEnd===normalized.length", block)
self.assertIn("!_smdMediaRefHasReliableBoundary(m[1])", block)
self.assertIn("unmatchedTail = candidate", block)