mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-30 11:52:04 +00:00
fix(session-db): enrich NULL session metadata via upsert instead of INSERT OR IGNORE
The gateway's get_or_create_session() creates a bare session row (source + user_id) before the agent exists. The agent's later create_session() carries the real model/model_config/system_prompt, but _insert_session_row used INSERT OR IGNORE — silently dropping that enrichment. Gateway sessions were left with NULL model and NULL billing metadata. Switch to INSERT ... ON CONFLICT(id) DO UPDATE with COALESCE so NULL columns get backfilled while values an earlier writer already set are never overwritten (a later bare write with source='unknown' can't clobber a real source/model). Credit: original report and fix direction by @LucidPaths (#5048).
This commit is contained in:
parent
61f56d27db
commit
23c03ced75
2 changed files with 55 additions and 3 deletions
|
|
@ -1486,14 +1486,35 @@ class SessionDB:
|
|||
parent_session_id: str = None,
|
||||
cwd: str = None,
|
||||
) -> None:
|
||||
"""Shared INSERT OR IGNORE for session rows."""
|
||||
"""Insert a session row, enriching NULL metadata on conflict.
|
||||
|
||||
The gateway's ``get_or_create_session`` creates a bare row (source +
|
||||
user_id) *before* the agent exists; the agent's later
|
||||
``create_session`` then carries the real ``model`` / ``model_config`` /
|
||||
``system_prompt``. A plain ``INSERT OR IGNORE`` silently dropped that
|
||||
enrichment, leaving gateway sessions with NULL model/billing metadata.
|
||||
The ``ON CONFLICT`` upsert backfills those fields via ``COALESCE`` —
|
||||
only filling columns that are still NULL, never overwriting values an
|
||||
earlier writer already set (so a later bare call with source="unknown"
|
||||
can't clobber a real source/model).
|
||||
"""
|
||||
def _do(conn):
|
||||
conn.execute(
|
||||
"""INSERT OR IGNORE INTO sessions (
|
||||
"""INSERT INTO sessions (
|
||||
id, source, user_id, session_key, chat_id, chat_type, thread_id,
|
||||
model, model_config, system_prompt, parent_session_id, cwd, started_at
|
||||
)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)""",
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
|
||||
ON CONFLICT(id) DO UPDATE SET
|
||||
model = COALESCE(sessions.model, excluded.model),
|
||||
model_config = COALESCE(sessions.model_config, excluded.model_config),
|
||||
system_prompt = COALESCE(sessions.system_prompt, excluded.system_prompt),
|
||||
session_key = COALESCE(sessions.session_key, excluded.session_key),
|
||||
chat_id = COALESCE(sessions.chat_id, excluded.chat_id),
|
||||
chat_type = COALESCE(sessions.chat_type, excluded.chat_type),
|
||||
thread_id = COALESCE(sessions.thread_id, excluded.thread_id),
|
||||
parent_session_id = COALESCE(sessions.parent_session_id, excluded.parent_session_id),
|
||||
cwd = COALESCE(sessions.cwd, excluded.cwd)""",
|
||||
(
|
||||
session_id,
|
||||
source,
|
||||
|
|
|
|||
|
|
@ -96,6 +96,37 @@ class TestSessionLifecycle:
|
|||
def test_get_nonexistent_session(self, db):
|
||||
assert db.get_session("nonexistent") is None
|
||||
|
||||
def test_create_session_enriches_null_metadata_on_conflict(self, db):
|
||||
"""Gateway creates a bare row first; the agent's later create_session
|
||||
must backfill model/model_config/system_prompt without clobbering the
|
||||
gateway's source/user_id/chat_id. Regression for NULL gateway metadata
|
||||
(sessions with NULL billing_provider/model)."""
|
||||
# Gateway bare row (source + user_id only), before the agent exists.
|
||||
db.create_session("s1", source="telegram", user_id="u1", chat_id="c1")
|
||||
bare = db.get_session("s1")
|
||||
assert bare["model"] is None
|
||||
# Agent enriches — passes source="cli" but real metadata.
|
||||
db.create_session(
|
||||
"s1", source="cli", model="claude-opus-4-6",
|
||||
model_config={"max_iterations": 90}, system_prompt="SYS",
|
||||
)
|
||||
enriched = db.get_session("s1")
|
||||
assert enriched["model"] == "claude-opus-4-6"
|
||||
assert enriched["system_prompt"] == "SYS"
|
||||
# Gateway-owned fields preserved (NOT clobbered by source="cli").
|
||||
assert enriched["source"] == "telegram"
|
||||
assert enriched["user_id"] == "u1"
|
||||
assert enriched["chat_id"] == "c1"
|
||||
|
||||
def test_create_session_does_not_overwrite_existing_metadata(self, db):
|
||||
"""A later bare write (source='unknown', model=...) must not overwrite
|
||||
a model/source an earlier writer already set."""
|
||||
db.create_session("s1", source="cli", model="real-model")
|
||||
db.create_session("s1", source="unknown", model="should-not-win")
|
||||
session = db.get_session("s1")
|
||||
assert session["model"] == "real-model"
|
||||
assert session["source"] == "cli"
|
||||
|
||||
def test_update_session_cwd_persists_git_branch(self, db):
|
||||
db.create_session(session_id="s1", source="cli")
|
||||
db.update_session_cwd("s1", "/work/repo", git_branch="pets-feature")
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue