hermes-admin/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 29 Packages Projects Releases Wiki Activity

fix(state): stop nulling the stored system prompt on route commits

Browse Source 
update_session_model (every /model commit), update_session_runtime_lock
and update_session_billing_route nulled the session's stored system
prompt snapshot unconditionally. The snapshot is a long-lived session's
provider cache prefix; nulling it makes the next turn take the broken-row
rebuild in _restore_or_build_system_prompt (the 'Stored system prompt
... is null' WARNING) and re-bills the whole prefix at ~0% cache.

The nulling predates _stored_prompt_matches_runtime, which now rebuilds
and re-persists exactly when the embedded Model:/Provider: footer is
stale and reuses the stored bytes otherwise. The DB-side nulling is
redundant for real switches and harmful for no-op route re-commits
(picker re-selects), runtime locks and billing-route writes.

Fixes #121840

(cherry picked from commit 07462e23c3b593d80546172a72f8385697c60c4c)
This commit is contained in:
Uttkarsh
2026-09-24 21:08:32 +05:30
committed by kshitij
parent 387450a357
commit ae6e4428cb
4 changed files with 219 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

31
hermes_state_sessions.py
View File

@@ -677,9 +677,17 @@ class SessionSessionsMixin:
self, session_id: str, model: str, provider: Optional[str] = None, *,
base_url: Optional[str] = None, api_mode: Optional[str] = None,
) -> None:
"""Set the model after a mid-session /model switch (unconditionally), null system_prompt so
stale Model:/Provider: footers rebuild, and drop any Browser runtime lock (lineage markers
survive).
"""Set the model after a mid-session /model switch (unconditionally) and drop any Browser
runtime lock (lineage markers survive).
The stored system prompt is deliberately PRESERVED: it is the session's provider cache
prefix, and a rebuilt prompt never matches those bytes — clearing the row re-bills the whole
prefix on a commit that may not even have moved the route (the picker re-committing the
session's current model), and the next turn then reads a NULL row and takes the broken-row
branch in ``agent.conversation_loop`` (a WARNING blaming the previous turn's
``update_system_prompt`` write path). The stale ``Model:``/``Provider:`` footer this used to
null the row for is caught by ``_stored_prompt_matches_runtime`` on the next turn: a real
switch rebuilds there (INFO, then re-persists), a same-route commit reuses the bytes.
When *provider* is given the whole route is written, in both shapes resume reads (top-level
keys for the TUI/Desktop, ``gateway_runtime`` for the CLI), so a later resume recombines the
@@ -698,8 +706,7 @@ class SessionSessionsMixin:
route = {"provider": provider, "base_url": base_url or None, "api_mode": api_mode or None}
patch.update(route, gateway_runtime=route)
self._write_model_config_patch(
session_id, patch, "UPDATE sessions SET model = ?, model_config = ?, "
"system_prompt = NULL, system_prompt_hash = NULL WHERE id = ?",
session_id, patch, "UPDATE sessions SET model = ?, model_config = ? WHERE id = ?",
lambda merged: (model, merged, session_id),
)
@@ -709,14 +716,12 @@ class SessionSessionsMixin:
params: Optional[Callable[[Optional[str]], tuple]] = None,
) -> None:
"""Merge ``patch`` into model_config then run ``sql`` with ``params(merged)`` in one write
transaction; no-op when the row doesn't exist. Custom ``sql`` (prompt-nulling) also GCs prompts."""
transaction; no-op when the row doesn't exist."""
def _do(conn):
merged = self._merge_model_config_json(conn, session_id, patch)
if merged is _MODEL_CONFIG_ROW_MISSING:
return
conn.execute(sql, params(merged) if params else (merged, session_id))
if params is not None:
self._delete_unreferenced_system_prompts(conn)
self._execute_write(_do)
def _merge_model_config_json(
@@ -755,8 +760,10 @@ class SessionSessionsMixin:
model_options: Optional[Dict[str, Any]] = None, route_source: Optional[str] = None,
confirmed: bool = False,
) -> None:
"""Persist a Browser / API-client runtime lock into model_config (lineage markers survive); null
system_prompt so cached footers cannot lie."""
"""Persist a Browser / API-client runtime lock into model_config (lineage markers survive).
The stored system prompt stays: a lock that moves the model is caught by the next turn's
``_stored_prompt_matches_runtime`` identity check, and one that does not must keep the
session's cache prefix byte-identical."""
lock = {
"provider": provider or "", "model": model or "", "model_options": model_options or {},
"route_source": route_source or "", "confirmed": bool(confirmed), "updated_at": time.time(),
@@ -765,9 +772,7 @@ class SessionSessionsMixin:
session_id, {"browser_model_lock": lock},
"""UPDATE sessions SET
model_config = ?,
model = COALESCE(?, model),
system_prompt = NULL,
system_prompt_hash = NULL
model = COALESCE(?, model)
WHERE id = ?""",
lambda merged: (merged, model, session_id),
)

12
hermes_state_usage.py
View File

@@ -86,8 +86,11 @@ class SessionUsageMixin:
self, session_id: str, *, provider: str, base_url: str, billing_mode: Optional[str] = None,
) -> None:
"""Unconditionally set the billing route (``update_token_counts`` only COALESCE-fills
NULLs) so the dashboard reflects the latest /model switch; also nulls
``system_prompt`` so the cached snapshot header is rebuilt.
NULLs) so the dashboard reflects the latest /model switch.
The stored system prompt is left alone — it is the session's cache prefix and the route
change is caught by the next turn's ``_stored_prompt_matches_runtime`` identity check, which
rebuilds only when the footer it embeds is actually stale.
See #48173, #48248.
"""
@@ -98,11 +101,8 @@ class SessionUsageMixin:
conn.execute("""UPDATE sessions SET
billing_provider = ?,
billing_base_url = ?,
billing_mode = COALESCE(?, billing_mode),
system_prompt = NULL,
system_prompt_hash = NULL
billing_mode = COALESCE(?, billing_mode)
WHERE id = ?""", (provider, base_url, billing_mode, session_id))
self._delete_unreferenced_system_prompts(conn)
self._execute_write(_do)
def queue_token_counts(self, session_id: str, **kwargs) -> None:

183
tests/agent/test_system_prompt_switch_invalidation.py Normal file
View File

@@ -0,0 +1,183 @@
"""A /model (or route) commit must not null a continuing session's stored system prompt.
Regression for the WARNING ``Stored system prompt for session X is null; rebuilding from
scratch this turn ... Investigate the previous turn's update_system_prompt write path``:
three switch-path writers cleared ``system_prompt``/``system_prompt_hash`` "so stale
Model:/Provider: footers rebuild" —
* ``SessionDB.update_session_model`` (every /model commit)
* ``SessionDB.update_session_runtime_lock`` (Browser / API-client lock)
* ``SessionDB.update_session_billing_route`` (billing route, also on provider fallbacks)
— so the next turn read a NULL row and took the broken-row branch of
``agent.conversation_loop._restore_or_build_system_prompt``: a WARNING blaming the previous
turn's ``update_system_prompt`` write path, plus a full prompt rebuild (the whole provider
prefix re-billed) even on a commit that never moved the route — the picker re-committing the
session's *current* model, which the production log shows verbatim
(``switched from deepseek-v4.1-flash to deepseek-v4.1-flash via hyper``).
The runtime-identity check (``_stored_prompt_matches_runtime``) is the single mechanism that
already rebuilds exactly when the stored footer is stale, so the writers preserve the row and
the check decides: a real switch rebuilds (INFO) on the next turn, a same-route commit reuses
the bytes verbatim, and the WARNING goes back to meaning "the stored prompt was lost".
Real ``SessionDB`` on a temp file; no mock stands in for the DB layer.
"""
from __future__ import annotations
import logging
from unittest.mock import MagicMock
import pytest
from agent.conversation_loop import _restore_or_build_system_prompt
from hermes_state import SessionDB
SESSION_ID = "switch-session"
def _stored_prompt(model: str, provider: str) -> str:
return (
"You are Hermes Agent.\n\n"
"Conversation started: Thursday, September 24, 2026\n"
f"Model: {model}\n"
f"Provider: {provider}"
)
@pytest.fixture()
def db(tmp_path, monkeypatch):
"""Real SessionDB on a temp state.db, isolated from the live HERMES_HOME."""
monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes-home"))
session_db = SessionDB(db_path=tmp_path / "state.db")
yield session_db
session_db.close()
def _make_agent(db, *, model: str, provider: str, prebuilt: str) -> MagicMock:
"""The minimal agent ``_restore_or_build_system_prompt`` needs; the DB is real."""
agent = MagicMock()
agent._cached_system_prompt = None
agent.session_id = SESSION_ID
agent.model = model
agent.provider = provider
agent.platform = "discord"
agent._session_db = db
agent._use_prompt_caching = False
agent._persist_disabled = True # no on_session_start hook, no tool-pin rewrite
agent.enabled_toolsets = agent.disabled_toolsets = None
agent.tools = []
agent._build_system_prompt = MagicMock(return_value=prebuilt)
return agent
def _continue_turn(db, *, model: str, provider: str, prebuilt: str, caplog):
"""Run the next turn of a continuing session and return (agent, warnings)."""
agent = _make_agent(db, model=model, provider=provider, prebuilt=prebuilt)
with caplog.at_level(logging.INFO, logger="agent.conversation_loop"):
_restore_or_build_system_prompt(agent, None, [{"role": "user", "content": "hi"}])
warnings = [r for r in caplog.records if r.levelno >= logging.WARNING]
return agent, warnings
class TestSwitchCommitsKeepTheStoredPrompt:
"""Each switch-path writer leaves the prompt snapshot (and its dedup row) in place."""
def _seed(self, db) -> str:
prompt = _stored_prompt("x-ai/grok-4.5", "nous")
db.create_session(SESSION_ID, source="discord", model="x-ai/grok-4.5")
db.update_system_prompt(SESSION_ID, prompt)
return prompt
def test_model_commit_keeps_the_snapshot(self, db):
prompt = self._seed(db)
db.update_session_model(
SESSION_ID, "anthropic/claude-opus-4.8", provider="anthropic", base_url="https://a/v1",
)
assert db.get_session(SESSION_ID)["system_prompt"] == prompt
# Content-addressed storage intact: the row still resolves through its hash.
raw = db._conn.execute(
"SELECT system_prompt, system_prompt_hash FROM sessions WHERE id = ?", (SESSION_ID,)
).fetchone()
assert raw["system_prompt"] is None
assert raw["system_prompt_hash"] is not None
assert db._conn.execute(
"SELECT COUNT(*) FROM system_prompts WHERE hash = ?", (raw["system_prompt_hash"],)
).fetchone()[0] == 1
def test_runtime_lock_keeps_the_snapshot(self, db):
prompt = self._seed(db)
db.update_session_runtime_lock(
SESSION_ID, model="anthropic/claude-opus-4.8", provider="anthropic", confirmed=True,
)
assert db.get_session(SESSION_ID)["system_prompt"] == prompt
def test_billing_route_keeps_the_snapshot(self, db):
prompt = self._seed(db)
db.update_session_billing_route(SESSION_ID, provider="openrouter", base_url="https://o/v1")
assert db.get_session(SESSION_ID)["system_prompt"] == prompt
class TestNextTurnAfterASwitchCommit:
def test_same_route_commit_reuses_the_stored_bytes(self, db, caplog):
"""The picker re-committing the session's current route must not cost a rebuild."""
prompt = _stored_prompt("glm-5.3", "zro")
db.create_session(SESSION_ID, source="discord", model="glm-5.3")
db.update_system_prompt(SESSION_ID, prompt)
db.update_session_model(SESSION_ID, "glm-5.3", provider="zro", base_url="https://hyper/v1")
agent, warnings = _continue_turn(
db, model="glm-5.3", provider="zro", prebuilt="REBUILT", caplog=caplog,
)
assert agent._cached_system_prompt == prompt
agent._build_system_prompt.assert_not_called()
assert warnings == []
def test_route_change_rebuilds_through_the_identity_check(self, db, caplog):
"""A real switch still rebuilds — on the next turn, via the stale footer, not a null row."""
db.create_session(SESSION_ID, source="discord", model="deepseek-v4.1-flash")
db.update_system_prompt(SESSION_ID, _stored_prompt("deepseek-v4.1-flash", "hyper"))
db.update_session_model(
SESSION_ID, "glm-5.3", provider="zro", base_url="https://hyper/v1",
)
rebuilt = _stored_prompt("glm-5.3", "zro")
agent, warnings = _continue_turn(
db, model="glm-5.3", provider="zro", prebuilt=rebuilt, caplog=caplog,
)
agent._build_system_prompt.assert_called_once()
assert agent._cached_system_prompt == rebuilt
# The rebuilt bytes are persisted so the following turns reuse them verbatim.
assert db.get_session(SESSION_ID)["system_prompt"] == rebuilt
assert warnings == []
assert any(
r.levelno == logging.INFO and "stale runtime identity" in r.getMessage()
for r in caplog.records
)
def test_a_genuinely_lost_prompt_still_warns(self, db, caplog):
"""The WARNING keeps its real meaning: nobody switched — the row was lost."""
db.create_session(SESSION_ID, source="discord", model="glm-5.3")
db.update_system_prompt(SESSION_ID, _stored_prompt("glm-5.3", "zro"))
db.update_system_prompt(SESSION_ID, None) # a write that lost the prompt
agent, warnings = _continue_turn(
db, model="glm-5.3", provider="zro", prebuilt="REBUILT", caplog=caplog,
)
assert agent._cached_system_prompt == "REBUILT"
assert any(
"is null; rebuilding" in w.getMessage() and "update_system_prompt write path" in w.getMessage()
for w in warnings
)

19
tests/hermes_cli/test_session_system_prompt_dedup.py
View File

@@ -59,7 +59,7 @@ def test_prompt_snapshots_are_deduplicated_and_hydrated_for_readers(db):
assert db.list_pending_handoffs()[0]["system_prompt"] == prompt
def test_prompt_replacement_and_route_changes_collect_only_orphans(db):
def test_route_changes_keep_the_prompt_and_replacement_collects_orphans(db):
shared_prompt = "Model: x-ai/grok-4.5\nProvider: nous"
db.create_session(
"s1",
@@ -70,6 +70,8 @@ def test_prompt_replacement_and_route_changes_collect_only_orphans(db):
)
db.create_session("s2", "cli", system_prompt=shared_prompt)
# A route change keeps the stored snapshot: it is the session's cache prefix, and the next
# turn's runtime-identity check rebuilds only when the footer it embeds is actually stale.
db.update_session_runtime_lock(
"s1",
model="anthropic/claude-opus-4.8",
@@ -77,7 +79,7 @@ def test_prompt_replacement_and_route_changes_collect_only_orphans(db):
confirmed=True,
)
s1 = db.get_session("s1")
assert s1["system_prompt"] is None
assert s1["system_prompt"] == shared_prompt
assert json.loads(s1["model_config"])["_branched_from"] == "parent"
assert db.get_session("s2")["system_prompt"] == shared_prompt
assert _prompt_count(db) == 1
@@ -87,13 +89,16 @@ def test_prompt_replacement_and_route_changes_collect_only_orphans(db):
provider="openrouter",
base_url="https://example.test/v1",
)
assert db.get_session("s2")["system_prompt"] is None
assert _prompt_count(db) == 0
assert db.get_session("s2")["system_prompt"] == shared_prompt
assert _prompt_count(db) == 1
db.update_system_prompt("s2", "replacement")
assert db.get_session("s2")["system_prompt"] == "replacement"
# Replacing the snapshot (a rebuild) is what orphans the previous bytes.
db.update_system_prompt("s1", "replacement")
assert db.get_session("s1")["system_prompt"] == "replacement"
assert _prompt_count(db) == 2 # "replacement" + the still-referenced shared prompt
db.update_system_prompt("s2", None)
assert _prompt_count(db) == 0
assert db.get_session("s2")["system_prompt"] is None
assert _prompt_count(db) == 1
def test_existing_session_enrichment_does_not_leak_unused_prompt(db):