Release v0.51.257 — Release HY (stage-r5) (#3598)
## Release v0.51.257 — Release HY (stage-r5) Two rebased ★★★ fixes. ### Fixed | Issue | Author | Fix | |-------|--------|-----| | #3546 | @rodboev | **"Refresh Models" on a provider card no longer returns "Error: Not found".** Sent `POST /api/models/refresh` but no route matched (404). Added the route, wired to the existing `invalidate_provider_models_cache(provider_id)`. | | #3548 | @franksong2702 | **Credential self-heal no longer writes to a dead `SessionDB` handle.** A credential-refresh evicted/closed the cached agent, but the retry rebuilt a new agent from kwargs still holding the old closed `SessionDB` → persistence silently targeted a dead handle. Per-request `SessionDB` construction centralized + refreshed on the retry. | ### Dropped from this batch - **#2660** (session-event SSE profile scoping) — the Codex regression gate found **two SILENT dropped-refresh bugs** the scoping introduced: (1) the `maxsize=1` subscriber-queue coalescing overwrites a pending profile-A event with a profile-B event → A-tabs filter B out and never refresh for the A change; (2) renamed-root/`default` alias mismatch (backend `_profiles_match` treats them equal, the client filter uses a literal `!==`). A dropped refresh (stale sidebar) is worse than the extra refreshes the PR removes. Held with `changes-requested` + repros + the fail-safe fix (coalesce to unscoped on a profile mismatch; normalize root aliases). ### Gate - Full pytest suite: **7622 passed, 0 failed** - ESLint: CLEAN · ruff: CLEAN · browser-smoke: CLEAN - Codex (regression): SHIP-ONLY-WITH-FIXES (#2660 dropped-refresh bugs) → #2660 dropped + held → **SAFE TO SHIP** Co-authored-by: rodboev <rodboev@users.noreply.github.com> Co-authored-by: franksong2702 <franksong2702@users.noreply.github.com>
This commit is contained in:
7 files changed
+267
-21
No files matched your search
@@ -3,6 +3,12 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
## [v0.51.257] — 2026-06-04 — Release HY (stage-r5 — provider refresh route + SessionDB self-heal)
|
||||
|
||||
### Fixed
|
||||
- **"Refresh Models" on a provider card no longer returns "Error: Not found".** The composer/Settings sent `POST /api/models/refresh` but no route matched, so it always 404'd. Added the route, wired to the existing `invalidate_provider_models_cache(provider_id)`. (#3546, @rodboev)
|
||||
- **Credential self-heal no longer writes to a dead `SessionDB` handle.** When the first request triggered a credential refresh, the cached agent was evicted/closed but the retry rebuilt a new agent from kwargs still holding the old, closed `SessionDB` — so persistence silently targeted a dead handle. Per-request `SessionDB` construction is now centralized and refreshed on the self-heal retry. (#3548, @franksong2702)
|
||||
|
||||
## [v0.51.256] — 2026-06-04 — Release HX (stage-r4 — bound WebUI memory growth and idle CPU on large installs)
|
||||
|
||||
### Fixed
|
||||
|
||||
@@ -6410,6 +6410,14 @@ def handle_post(handler, parsed) -> bool:
|
||||
return bad(handler, result.get("error", "Unknown error"))
|
||||
return j(handler, result)
|
||||
|
||||
if parsed.path == "/api/models/refresh":
|
||||
provider_id = (body.get("provider") or "").strip().lower()
|
||||
if not provider_id:
|
||||
return bad(handler, "provider is required")
|
||||
from api.config import invalidate_provider_models_cache
|
||||
invalidate_provider_models_cache(provider_id)
|
||||
return j(handler, {"ok": True, "provider": provider_id})
|
||||
|
||||
if parsed.path == "/api/reasoning":
|
||||
# CLI-parity /reasoning handler — writes to the same config.yaml keys
|
||||
# the CLI uses (display.show_reasoning, agent.reasoning_effort) so a
|
||||
|
||||
+34
-7
@@ -3760,6 +3760,36 @@ def _last_resort_sync_from_core(session, stream_id, agent_lock):
|
||||
)
|
||||
|
||||
|
||||
def _build_session_db_for_stream(state_db_path):
|
||||
"""Build a per-request SessionDB handle for WebUI session search.
|
||||
|
||||
Returns ``None`` if the helper module or constructor fails so callers can
|
||||
continue without session_search rather than propagating a hard failure.
|
||||
"""
|
||||
try:
|
||||
from hermes_state import SessionDB
|
||||
return SessionDB(db_path=state_db_path)
|
||||
except Exception as _db_err:
|
||||
print(f"[webui] WARNING: SessionDB init failed - session_search will be unavailable: {_db_err}", flush=True)
|
||||
return None
|
||||
|
||||
|
||||
def _replace_session_db_in_kwargs(agent_kwargs, state_db_path):
|
||||
"""Build a fresh SessionDB and replace ``agent_kwargs['session_db']`` safely."""
|
||||
if not isinstance(agent_kwargs, dict):
|
||||
return None
|
||||
|
||||
_old_session_db = agent_kwargs.get("session_db")
|
||||
_next_session_db = _build_session_db_for_stream(state_db_path)
|
||||
if _old_session_db is not None and _old_session_db is not _next_session_db:
|
||||
try:
|
||||
_old_session_db.close()
|
||||
except Exception:
|
||||
logger.debug("Failed to close previous session_db handle during self-heal")
|
||||
agent_kwargs["session_db"] = _next_session_db
|
||||
return _next_session_db
|
||||
|
||||
|
||||
def _attempt_credential_self_heal(
|
||||
provider_id, session_id, _agent_lock_ref,
|
||||
):
|
||||
@@ -4931,13 +4961,8 @@ def _run_agent_streaming(
|
||||
raise ImportError(_aiagent_import_error_detail())
|
||||
|
||||
# Initialize SessionDB so session_search works in WebUI sessions
|
||||
_session_db = None
|
||||
try:
|
||||
from hermes_state import SessionDB
|
||||
_state_db_path = (Path(_profile_home) / "state.db") if _profile_home else None
|
||||
_session_db = SessionDB(db_path=_state_db_path)
|
||||
except Exception as _db_err:
|
||||
print(f"[webui] WARNING: SessionDB init failed — session_search will be unavailable: {_db_err}", flush=True)
|
||||
_state_db_path = (Path(_profile_home) / "state.db") if _profile_home else None
|
||||
_session_db = _build_session_db_for_stream(_state_db_path)
|
||||
resolved_model, resolved_provider, resolved_base_url = resolve_model_provider(
|
||||
model_with_provider_context(model, provider_context)
|
||||
)
|
||||
@@ -5710,6 +5735,7 @@ def _run_agent_streaming(
|
||||
_agent_kwargs['base_url'] = resolved_base_url
|
||||
_agent_kwargs['model'] = resolved_model
|
||||
_agent_kwargs['provider'] = resolved_provider
|
||||
_replace_session_db_in_kwargs(_agent_kwargs, _state_db_path)
|
||||
if 'credential_pool' in _agent_params:
|
||||
_agent_kwargs['credential_pool'] = _heal_rt.get('credential_pool')
|
||||
agent = _AIAgent(**_agent_kwargs)
|
||||
@@ -6775,6 +6801,7 @@ def _run_agent_streaming(
|
||||
_heal_kwargs['base_url'] = resolved_base_url
|
||||
_heal_kwargs['model'] = resolved_model
|
||||
_heal_kwargs['provider'] = resolved_provider
|
||||
_replace_session_db_in_kwargs(_heal_kwargs, _state_db_path)
|
||||
if 'credential_pool' in _agent_params:
|
||||
_heal_kwargs['credential_pool'] = _heal_rt.get('credential_pool')
|
||||
_heal_agent = _AIAgent(**_heal_kwargs)
|
||||
|
||||
+2
-1
@@ -7209,11 +7209,12 @@ async function _refreshProviderModels(providerId, btn){
|
||||
const res=await api('/api/models/refresh',{method:'POST',body:JSON.stringify({provider:providerId})});
|
||||
if(res.ok){
|
||||
showToast(t('providers_models_refreshed')||('Models refreshed for '+res.provider));
|
||||
_refreshModelDropdownsAfterProviderChange();
|
||||
}else{
|
||||
showToast(res.error||'Failed to refresh models');
|
||||
}
|
||||
}catch(e){
|
||||
showToast('Error: '+e.message);
|
||||
showToast(e.status===404?'Refresh not available for this provider.':(e.message||'Failed to refresh models'));
|
||||
}finally{
|
||||
btn.disabled=false;
|
||||
btn.innerHTML=orig;
|
||||
|
||||
@@ -0,0 +1,112 @@
|
||||
"""Regression tests for #3546 — POST /api/models/refresh must exist and
|
||||
invalidate the per-provider model cache so the "Refresh Models" button
|
||||
on Settings > Providers works instead of returning 404.
|
||||
|
||||
The bug
|
||||
-------
|
||||
``static/panels.js`` sends ``POST /api/models/refresh`` from
|
||||
``_refreshProviderModels``, but no route handled that path in
|
||||
``api/routes.py``. Every click showed "Error: Not found" because the
|
||||
server returned 404, which ``workspace.js``'s ``api()`` helper surfaced
|
||||
as an error toast.
|
||||
|
||||
The fix
|
||||
-------
|
||||
``api/routes.py`` adds a ``POST /api/models/refresh`` branch that calls
|
||||
``invalidate_provider_models_cache(provider_id)`` and returns
|
||||
``{"ok": true, "provider": provider_id}``. The frontend success path
|
||||
now also calls ``_refreshModelDropdownsAfterProviderChange()`` so the
|
||||
model picker rebuilds immediately.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
|
||||
|
||||
REPO = Path(__file__).resolve().parent.parent
|
||||
|
||||
|
||||
def _read_static(name: str) -> str:
|
||||
return (REPO / "static" / name).read_text(encoding="utf-8")
|
||||
|
||||
|
||||
def _extract_function_body(src: str, signature: str) -> str:
|
||||
"""Return the source of a top-level function declaration via brace-balance."""
|
||||
idx = src.find(signature)
|
||||
if idx == -1:
|
||||
raise AssertionError(f"signature {signature!r} not found in source")
|
||||
open_idx = src.find("{", idx)
|
||||
if open_idx == -1:
|
||||
raise AssertionError(f"could not find opening brace after {signature!r}")
|
||||
depth = 0
|
||||
for i in range(open_idx, len(src)):
|
||||
c = src[i]
|
||||
if c == "{":
|
||||
depth += 1
|
||||
elif c == "}":
|
||||
depth -= 1
|
||||
if depth == 0:
|
||||
return src[idx : i + 1]
|
||||
raise AssertionError(f"unbalanced braces in {signature!r}")
|
||||
|
||||
|
||||
class TestRefreshRouteExists:
|
||||
"""The POST /api/models/refresh route must exist in routes.py and follow
|
||||
the same input validation pattern as /api/providers/delete."""
|
||||
|
||||
def test_route_branch_present(self):
|
||||
src = (REPO / "api" / "routes.py").read_text(encoding="utf-8")
|
||||
assert '"/api/models/refresh"' in src, (
|
||||
"POST /api/models/refresh route missing from api/routes.py. "
|
||||
"Without it, the Refresh Models button 404s (#3546)."
|
||||
)
|
||||
|
||||
def test_route_validates_provider_param(self):
|
||||
src = (REPO / "api" / "routes.py").read_text(encoding="utf-8")
|
||||
idx = src.find('"/api/models/refresh"')
|
||||
block = src[idx : idx + 500]
|
||||
assert "provider" in block and "bad(handler" in block, (
|
||||
"/api/models/refresh must validate that 'provider' is present "
|
||||
"and return 400 via bad() when missing."
|
||||
)
|
||||
|
||||
def test_route_calls_invalidate_provider_models_cache(self):
|
||||
src = (REPO / "api" / "routes.py").read_text(encoding="utf-8")
|
||||
idx = src.find('"/api/models/refresh"')
|
||||
block = src[idx : idx + 500]
|
||||
assert "invalidate_provider_models_cache" in block, (
|
||||
"/api/models/refresh must call invalidate_provider_models_cache "
|
||||
"to bust the per-provider TTL cache."
|
||||
)
|
||||
|
||||
def test_route_returns_ok_with_provider(self):
|
||||
src = (REPO / "api" / "routes.py").read_text(encoding="utf-8")
|
||||
idx = src.find('"/api/models/refresh"')
|
||||
block = src[idx : idx + 500]
|
||||
assert '"ok"' in block and '"provider"' in block, (
|
||||
"/api/models/refresh must return {ok: true, provider: provider_id} "
|
||||
"so the frontend can confirm the operation succeeded."
|
||||
)
|
||||
|
||||
|
||||
class TestFrontendRefreshPath:
|
||||
"""The frontend success path must update the model picker immediately
|
||||
after a cache bust, not wait for the next /api/models poll."""
|
||||
|
||||
def test_refresh_calls_dropdown_updater(self):
|
||||
src = _read_static("panels.js")
|
||||
body = _extract_function_body(src, "async function _refreshProviderModels(")
|
||||
assert "_refreshModelDropdownsAfterProviderChange()" in body, (
|
||||
"_refreshProviderModels must call _refreshModelDropdownsAfterProviderChange() "
|
||||
"on success so the model picker rebuilds immediately after a cache "
|
||||
"bust instead of waiting for the next /api/models call (#3546)."
|
||||
)
|
||||
|
||||
def test_refresh_shows_friendly_404(self):
|
||||
src = _read_static("panels.js")
|
||||
body = _extract_function_body(src, "async function _refreshProviderModels(")
|
||||
assert "e.status===404" in body or "e.status === 404" in body, (
|
||||
"_refreshProviderModels catch block must check e.status===404 to "
|
||||
"show a friendly message when the route is missing on older backends."
|
||||
)
|
||||
@@ -0,0 +1,85 @@
|
||||
"""Regression tests for issue #3548: closed SessionDB handles are not reused on self-heal retry."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
import sys
|
||||
import types
|
||||
from unittest import mock
|
||||
|
||||
def test_session_db_helper_uses_request_state_db_path():
|
||||
import api.streaming as streaming
|
||||
|
||||
calls = {}
|
||||
|
||||
class FakeSessionDB:
|
||||
def __init__(self, db_path=None):
|
||||
calls["db_path"] = db_path
|
||||
|
||||
def close(self):
|
||||
calls["closed"] = True
|
||||
|
||||
fake_state = types.ModuleType("hermes_state")
|
||||
fake_state.SessionDB = FakeSessionDB
|
||||
|
||||
with mock.patch.dict(sys.modules, {"hermes_state": fake_state}):
|
||||
state_db_path = Path("/tmp/profile") / "state.db"
|
||||
db = streaming._build_session_db_for_stream(state_db_path)
|
||||
|
||||
assert db is not None
|
||||
assert calls["db_path"] == state_db_path
|
||||
assert isinstance(db, FakeSessionDB)
|
||||
|
||||
|
||||
def test_session_db_helper_returns_none_when_constructor_fails():
|
||||
import api.streaming as streaming
|
||||
|
||||
def failing_session_db(db_path=None):
|
||||
raise RuntimeError("SessionDB unavailable")
|
||||
|
||||
fake_state = types.ModuleType("hermes_state")
|
||||
fake_state.SessionDB = mock.Mock(side_effect=failing_session_db)
|
||||
|
||||
with mock.patch.dict(sys.modules, {"hermes_state": fake_state}):
|
||||
db = streaming._build_session_db_for_stream(Path("/tmp/profile/state.db"))
|
||||
|
||||
assert db is None
|
||||
|
||||
|
||||
def test_self_heal_session_db_handle_is_replaced_safely():
|
||||
import api.streaming as streaming
|
||||
|
||||
class FakeDb:
|
||||
def __init__(self, label):
|
||||
self.label = label
|
||||
|
||||
def close(self):
|
||||
self.closed = True
|
||||
|
||||
old_db = FakeDb("old")
|
||||
new_db = FakeDb("new")
|
||||
with mock.patch.object(
|
||||
streaming, "_build_session_db_for_stream", return_value=new_db
|
||||
) as build_db:
|
||||
kwargs = {"session_db": old_db}
|
||||
assigned_db = streaming._replace_session_db_in_kwargs(kwargs, Path("/tmp/profile/state.db"))
|
||||
|
||||
assert assigned_db is new_db
|
||||
assert kwargs["session_db"] is new_db
|
||||
build_db.assert_called_once_with(Path("/tmp/profile/state.db"))
|
||||
assert getattr(old_db, "closed", False) is True
|
||||
assert hasattr(new_db, "closed") is False
|
||||
|
||||
|
||||
def test_session_db_handle_not_double_closed_when_rebuilt_to_same_instance():
|
||||
import api.streaming as streaming
|
||||
|
||||
db = mock.Mock(name="session_db")
|
||||
|
||||
with mock.patch.object(streaming, "_build_session_db_for_stream", return_value=db):
|
||||
kwargs = {"session_db": db}
|
||||
returned_db = streaming._replace_session_db_in_kwargs(kwargs, Path("/tmp/profile/state.db"))
|
||||
|
||||
assert returned_db is db
|
||||
assert kwargs["session_db"] is db
|
||||
db.close.assert_not_called()
|
||||
+20
-13
@@ -56,17 +56,19 @@ class TestSessionDBInjection(unittest.TestCase):
|
||||
|
||||
def test_sessiondb_init_in_try_except(self):
|
||||
"""SessionDB init must be wrapped in try/except for non-fatal failure handling."""
|
||||
# Check that the try/except pattern surrounding SessionDB(db_path=...) is present.
|
||||
# Check that SessionDB init is wrapped in the helper used by streaming.
|
||||
helper_start = STREAMING_PY.find("def _build_session_db_for_stream")
|
||||
helper_end = STREAMING_PY.find("\n\ndef _attempt_credential_self_heal", helper_start)
|
||||
self.assertGreater(helper_start, -1, "session DB helper missing in streaming.py")
|
||||
helper_src = STREAMING_PY[helper_start:helper_end]
|
||||
pattern = (
|
||||
r"try:\s*\n"
|
||||
r"\s*from hermes_state import SessionDB\s*\n"
|
||||
r'\s*_state_db_path\s*=\s*\(Path\(_profile_home\)\s*/\s*"state\.db"\)\s*if\s*_profile_home\s*else\s*None\s*\n'
|
||||
r"\s*_session_db\s*=\s*SessionDB\(db_path=_state_db_path\)"
|
||||
r"def _build_session_db_for_stream"
|
||||
r"[\s\S]*?try:\s*\n[\s\S]*?from hermes_state import SessionDB[\s\S]*?return SessionDB\(db_path=state_db_path\)[\s\S]*?except Exception as _db_err:"
|
||||
)
|
||||
self.assertRegex(
|
||||
STREAMING_PY,
|
||||
helper_src,
|
||||
pattern,
|
||||
"SessionDB(db_path=...) init must be inside a try block for non-fatal error handling",
|
||||
"SessionDB init helper must use try/except for non-fatal error handling",
|
||||
)
|
||||
|
||||
def test_sessiondb_failure_logs_warning(self):
|
||||
@@ -88,13 +90,18 @@ class TestSessionDBInjection(unittest.TestCase):
|
||||
)
|
||||
|
||||
def test_session_db_default_is_none(self):
|
||||
"""_session_db must be initialized to None before the try block (safe default)."""
|
||||
# Pattern: _session_db = None followed (eventually) by the try/SessionDB block
|
||||
pattern = r"_session_db\s*=\s*None\s*\n\s*try:"
|
||||
self.assertRegex(
|
||||
STREAMING_PY,
|
||||
"""SessionDB should now be initialized through the helper call."""
|
||||
pattern = "_state_db_path = (Path(_profile_home) / \"state.db\") if _profile_home else None"
|
||||
helper_pattern = "_session_db = _build_session_db_for_stream(_state_db_path)"
|
||||
self.assertIn(
|
||||
pattern,
|
||||
"_session_db must default to None before try/except block (PR #356)",
|
||||
STREAMING_PY,
|
||||
"_state_db_path should be resolved from profile home in streaming.py",
|
||||
)
|
||||
self.assertIn(
|
||||
helper_pattern,
|
||||
STREAMING_PY,
|
||||
"_session_db should be initialized via _build_session_db_for_stream in streaming.py",
|
||||
)
|
||||
|
||||
|
||||
|
||||
Reference in new issue
Block a user