From ef1c622105a3955f1b66903bf25def8686181531 Mon Sep 17 00:00:00 2001 From: Thatgfsj Date: Thu, 16 Jul 2026 20:29:11 -0700 Subject: [PATCH] fix(title): prevent stale background title generation from reloading unloaded Ollama models MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a runtime_validator callback to generate_title() / auto_title_session() / maybe_auto_title(). Callers snapshot the session's model+provider when spawning the background titler; the validator runs right before the LLM request and skips it silently when the live runtime no longer matches — so a stale title request can't reload a model that strict_single_load already evicted after a user model switch. Fail-open: a raising validator never disables titling. Wired at all four call sites (cli, gateway, tui_gateway, acp_adapter). Surgical reapply of PR #19137 (base was 8k+ commits stale; the original patch predates the pinned-language prompts, the atomic-write helper, and the moved TUI/ACP call sites). Original work by @Thatgfsj. Closes #19027. --- acp_adapter/server.py | 9 +++++ agent/title_generator.py | 34 +++++++++++++++- cli.py | 10 +++++ gateway/run.py | 10 +++++ scripts/release.py | 1 + tests/agent/test_title_generator.py | 63 +++++++++++++++++++++++++++++ tui_gateway/server.py | 9 +++++ 7 files changed, 135 insertions(+), 1 deletion(-) diff --git a/acp_adapter/server.py b/acp_adapter/server.py index 7646f332534..d86e4065186 100644 --- a/acp_adapter/server.py +++ b/acp_adapter/server.py @@ -1617,6 +1617,11 @@ class HermesACPAgent(acp.Agent): self._send_session_info_update(session_id), ) + # Snapshot the runtime identity; the validator lets the + # background titler skip its LLM call if the session's model + # changed before it fires (#19027). + _title_model = getattr(state.agent, "model", None) + _title_provider = getattr(state.agent, "provider", None) maybe_auto_title( self.session_manager._get_db(), session_id, @@ -1630,6 +1635,10 @@ class HermesACPAgent(acp.Agent): "api_key": getattr(state.agent, "api_key", None), "api_mode": getattr(state.agent, "api_mode", None), }, + runtime_validator=lambda: ( + getattr(state.agent, "model", None) == _title_model + and getattr(state.agent, "provider", None) == _title_provider + ), title_callback=_notify_title_update, ) except Exception: diff --git a/agent/title_generator.py b/agent/title_generator.py index 9dcdf1e8963..7469a665bfc 100644 --- a/agent/title_generator.py +++ b/agent/title_generator.py @@ -19,6 +19,12 @@ logger = logging.getLogger(__name__) FailureCallback = Callable[[str, BaseException], None] TitleCallback = Callable[[str], None] +# Validation callback: () -> bool. Called right before the LLM request in +# generate_title(). Return False to skip — e.g. the user switched models +# after this background thread captured its runtime snapshot, and sending +# the request would reload a model the runtime already evicted (#19027). +RuntimeValidator = Callable[[], bool] + _TITLE_PROMPT = ( "Generate a short, descriptive title (3-7 words) for a conversation that starts with the " "following exchange. The title should capture the main topic or intent. " @@ -71,6 +77,7 @@ def generate_title( timeout: Optional[float] = None, failure_callback: Optional[FailureCallback] = None, main_runtime: dict = None, + runtime_validator: Optional[RuntimeValidator] = None, ) -> Optional[str]: """Generate a session title from the first exchange. @@ -82,11 +89,26 @@ def generate_title( auxiliary call raises — the caller typically wires this to ``AIAgent._emit_auxiliary_failure`` so the user sees a warning instead of silently accumulating untitled sessions. + + ``runtime_validator`` is called right before the LLM request. If it + returns False (e.g. the user's model was switched since the background + thread captured its runtime snapshot), the call is skipped silently — + no request is sent, so a stale title request can't reload a model the + runtime already unloaded (#19027). """ if not _auto_title_enabled(): logger.debug("Auto-title skipped: auxiliary.title_generation.enabled=false") return None + if runtime_validator is not None: + try: + if not runtime_validator(): + logger.debug("Title generation skipped: runtime validator returned False") + return None + except Exception: + # Fail open: a broken validator must not disable titling. + logger.debug("Title runtime validator raised; proceeding", exc_info=True) + # Truncate long messages to keep the request small user_snippet = user_message[:500] if user_message else "" assistant_snippet = assistant_response[:500] if assistant_response else "" @@ -193,6 +215,7 @@ def auto_title_session( failure_callback: Optional[FailureCallback] = None, main_runtime: dict = None, title_callback: Optional[TitleCallback] = None, + runtime_validator: Optional[RuntimeValidator] = None, ) -> None: """Generate and set a session title if one doesn't already exist. @@ -201,6 +224,7 @@ def auto_title_session( - session_db is None - session already has a title (user-set or previously auto-generated) - title generation fails + - runtime_validator returns False (model was switched) Never lets an exception escape: this is a daemon-thread target, and an escaping exception would spray a raw traceback into the user's terminal @@ -220,6 +244,7 @@ def auto_title_session( failure_callback=failure_callback, main_runtime=main_runtime, title_callback=title_callback, + runtime_validator=runtime_validator, ) except Exception as e: # WARNING (not debug) so operators see it in agent.log; the message @@ -245,6 +270,7 @@ def _auto_title_session( failure_callback: Optional[FailureCallback] = None, main_runtime: dict = None, title_callback: Optional[TitleCallback] = None, + runtime_validator: Optional[RuntimeValidator] = None, ) -> None: """Body of :func:`auto_title_session` — see its docstring.""" if not session_db or not session_id: @@ -278,7 +304,11 @@ def _auto_title_session( set_accounting_context(session_db, session_id) title = generate_title( - user_message, assistant_response, failure_callback=failure_callback, main_runtime=main_runtime + user_message, + assistant_response, + failure_callback=failure_callback, + main_runtime=main_runtime, + runtime_validator=runtime_validator, ) if not title: return @@ -306,6 +336,7 @@ def maybe_auto_title( failure_callback: Optional[FailureCallback] = None, main_runtime: dict = None, title_callback: Optional[TitleCallback] = None, + runtime_validator: Optional[RuntimeValidator] = None, ) -> None: """Fire-and-forget title generation after the first exchange. @@ -337,6 +368,7 @@ def maybe_auto_title( "failure_callback": failure_callback, "main_runtime": main_runtime, "title_callback": title_callback, + "runtime_validator": runtime_validator, }, daemon=True, name="auto-title", diff --git a/cli.py b/cli.py index 8184f98d392..8ed35226fb6 100644 --- a/cli.py +++ b/cli.py @@ -12686,6 +12686,12 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin): _title_failure_cb = getattr( self.agent, "_emit_auxiliary_failure", None ) if self.agent else None + # Snapshot the runtime identity; the validator lets the + # background titler skip its LLM call if the user switches + # models before it fires (a stale request would reload an + # unloaded Ollama model, #19027). + _title_model = self.model + _title_provider = self.provider maybe_auto_title( self._session_db, self.session_id, @@ -12700,6 +12706,10 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin): "api_key": self.api_key, "api_mode": self.api_mode, }, + runtime_validator=lambda: ( + getattr(self, "model", None) == _title_model + and getattr(self, "provider", None) == _title_provider + ), ) except Exception: pass diff --git a/gateway/run.py b/gateway/run.py index e5ca409706a..9e2b4343d5b 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -19869,6 +19869,12 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew "Gateway auto-title failure suppressed (not user-visible): %s: %s", task, exc, ) + # Snapshot the runtime identity; the validator lets the + # background titler skip its LLM call if the session's + # model changed before it fires (a stale request would + # reload an unloaded Ollama model, #19027). + _title_model = getattr(agent, "model", None) if agent else None + _title_provider = getattr(agent, "provider", None) if agent else None maybe_auto_title_kwargs = { "failure_callback": _title_failure_cb, "main_runtime": { @@ -19878,6 +19884,10 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew "api_key": getattr(agent, "api_key", None), "api_mode": getattr(agent, "api_mode", None), } if agent else None, + "runtime_validator": (lambda: ( + getattr(agent, "model", None) == _title_model + and getattr(agent, "provider", None) == _title_provider + )) if agent else None, } if self._is_telegram_topic_lane(source): maybe_auto_title_kwargs["title_callback"] = lambda title: self._schedule_telegram_topic_title_rename( diff --git a/scripts/release.py b/scripts/release.py index 431cc41fe54..9a217938b3a 100755 --- a/scripts/release.py +++ b/scripts/release.py @@ -369,6 +369,7 @@ AUTHOR_MAP = { "dirtyren@users.noreply.github.com": "dirtyren", "s96919@gmail.com": "s96919", "rasitakyol@hotmail.com": "rasitakyol", + "thatgfsj@gmail.com": "Thatgfsj", "141703117+seagpt@users.noreply.github.com": "seagpt", "yakimenkoleksander228@gmail.com": "doxe0x", "a54983334@163.com": "Code-suphub", diff --git a/tests/agent/test_title_generator.py b/tests/agent/test_title_generator.py index bb0c9627a16..321f9030ae0 100644 --- a/tests/agent/test_title_generator.py +++ b/tests/agent/test_title_generator.py @@ -387,6 +387,7 @@ class TestMaybeAutoTitle: failure_callback=None, main_runtime=None, title_callback=None, + runtime_validator=None, ) def test_skips_when_title_generation_disabled(self): @@ -430,6 +431,7 @@ class TestMaybeAutoTitle: failure_callback=_cb, main_runtime=None, title_callback=None, + runtime_validator=None, ) def test_skips_if_no_response(self): @@ -513,3 +515,64 @@ class TestAutoTitleDuplicateHandling: db.set_session_title.return_value = False with pytest.raises(RuntimeError): _persist_session_title(db, "missing", "Some Title") + + +class TestRuntimeValidator: + """runtime_validator gating (#19027): a stale background title request + must not fire when the session's model/provider changed after spawn.""" + + def test_skips_when_validator_returns_false(self): + with patch("agent.title_generator.call_llm") as mock_llm: + title = generate_title( + "question", "answer", + runtime_validator=lambda: False, + ) + assert title is None + mock_llm.assert_not_called() + + def test_allows_when_validator_returns_true(self): + mock_response = MagicMock() + mock_response.choices = [MagicMock()] + mock_response.choices[0].message.content = "Validated Title" + + with patch("agent.title_generator.call_llm", return_value=mock_response) as mock_llm: + title = generate_title( + "question", "answer", + runtime_validator=lambda: True, + ) + assert title == "Validated Title" + mock_llm.assert_called_once() + + def test_broken_validator_fails_open(self): + mock_response = MagicMock() + mock_response.choices = [MagicMock()] + mock_response.choices[0].message.content = "Resilient Title" + + def _bad_validator(): + raise RuntimeError("validator gone") + + with patch("agent.title_generator.call_llm", return_value=mock_response) as mock_llm: + title = generate_title( + "question", "answer", + runtime_validator=_bad_validator, + ) + assert title == "Resilient Title" + mock_llm.assert_called_once() + + def test_forwards_runtime_validator_to_worker(self): + db = MagicMock() + db.get_session_title.return_value = None + history = [ + {"role": "user", "content": "hello"}, + {"role": "assistant", "content": "hi there"}, + ] + + def _v(): + return True + + with patch("agent.title_generator.auto_title_session") as mock_auto: + maybe_auto_title(db, "sess-1", "hello", "hi there", history, runtime_validator=_v) + import time + time.sleep(0.3) + kwargs = mock_auto.call_args.kwargs + assert kwargs["runtime_validator"] is _v diff --git a/tui_gateway/server.py b/tui_gateway/server.py index 3e405268d84..7f1a7ffe374 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -9678,6 +9678,11 @@ def _run_prompt_submit(rid, sid: str, session: dict, text: Any) -> None: from agent.title_generator import maybe_auto_title _title_key = session.get("session_key") or sid + # Snapshot the runtime identity; the validator lets the + # background titler skip its LLM call if the session's + # model changed before it fires (#19027). + _title_model = getattr(agent, "model", None) + _title_provider = getattr(agent, "provider", None) maybe_auto_title( _get_db(), _title_key, @@ -9695,6 +9700,10 @@ def _run_prompt_submit(rid, sid: str, session: dict, text: Any) -> None: "api_key": getattr(agent, "api_key", None), "api_mode": getattr(agent, "api_mode", None), }, + runtime_validator=lambda: ( + getattr(agent, "model", None) == _title_model + and getattr(agent, "provider", None) == _title_provider + ), # Push the generated title live so the sidebar renames # without waiting for the next list refresh (the titler # runs async, after this turn's refresh already fired).