feat(sessions): one-shot single-match owner backfill for legacy NULL-profile rows (#94724)
POST /api/sessions/owner-backfill stamps a store's own serving-profile identity onto its pre-#95407 'profile_name = NULL' session rows. Single match by construction (each profile's state.db belongs to exactly one profile), idempotent, one-shot-per-row, never overwrites a non-NULL owner, and reports the stamped count for logging. Refs #94724
This commit is contained in:
@@ -345,6 +345,17 @@ class SessionRename(BaseModel):
|
|||||||
profile: Optional[str] = None
|
profile: Optional[str] = None
|
||||||
|
|
||||||
|
|
||||||
|
class SessionOwnerBackfill(BaseModel):
|
||||||
|
"""Body for POST /api/sessions/owner-backfill (#94724 legacy migration).
|
||||||
|
|
||||||
|
``profile`` scopes WHICH profile's state.db is stamped (same semantics as
|
||||||
|
every other session route); the stamped value is always that store's own
|
||||||
|
serving-profile identity — the caller cannot inject an arbitrary owner.
|
||||||
|
"""
|
||||||
|
|
||||||
|
profile: Optional[str] = None
|
||||||
|
|
||||||
|
|
||||||
# --- from web_server.py (originally lines 12149-12174) ---
|
# --- from web_server.py (originally lines 12149-12174) ---
|
||||||
|
|
||||||
class SessionPrune(BaseModel):
|
class SessionPrune(BaseModel):
|
||||||
|
|||||||
@@ -26,6 +26,7 @@ from hermes_cli.web_deps import late
|
|||||||
from hermes_cli.web_models import (
|
from hermes_cli.web_models import (
|
||||||
BulkDeleteSessions,
|
BulkDeleteSessions,
|
||||||
SessionImport,
|
SessionImport,
|
||||||
|
SessionOwnerBackfill,
|
||||||
SessionPrune,
|
SessionPrune,
|
||||||
SessionRename,
|
SessionRename,
|
||||||
)
|
)
|
||||||
@@ -702,6 +703,51 @@ async def delete_session_endpoint(session_id: str, profile: Optional[str] = None
|
|||||||
return await asyncio.to_thread(_delete)
|
return await asyncio.to_thread(_delete)
|
||||||
|
|
||||||
|
|
||||||
|
@manage_router.post("/api/sessions/owner-backfill")
|
||||||
|
async def backfill_session_owner_profiles(body: SessionOwnerBackfill):
|
||||||
|
"""Stamp legacy ``profile_name = NULL`` session rows with this store's own
|
||||||
|
serving-profile identity (#94724 legacy-session migration).
|
||||||
|
|
||||||
|
Pre-#95407 rows never recorded an owning profile. That was fine while one
|
||||||
|
backend served everything, but a Desktop with registry topology (≥2
|
||||||
|
registered connections) fails closed on unowned rows by design — leaving
|
||||||
|
every pre-campaign session unresumable with no migration path. Each
|
||||||
|
profile's ``state.db`` belongs to exactly one profile, so stamping that
|
||||||
|
store's own name is a single-match backfill, never a guess; the value
|
||||||
|
written is the SAME serving-profile identity the list endpoints already
|
||||||
|
stamp onto outgoing rows (``row_profile`` in ``get_sessions``). Idempotent
|
||||||
|
and one-shot-per-row: non-NULL owners are never overwritten and a second
|
||||||
|
call reports 0.
|
||||||
|
"""
|
||||||
|
profile_name: Optional[str] = None
|
||||||
|
if body.profile:
|
||||||
|
profile_name, _ = _cron_profile_home(body.profile)
|
||||||
|
stamp = profile_name or _cron_default_profile()
|
||||||
|
|
||||||
|
def _backfill():
|
||||||
|
db = _open_session_db_for_profile(body.profile, read_only=False)
|
||||||
|
try:
|
||||||
|
return db.backfill_null_session_profiles(stamp)
|
||||||
|
finally:
|
||||||
|
db.close()
|
||||||
|
|
||||||
|
try:
|
||||||
|
stamped = await asyncio.to_thread(_backfill)
|
||||||
|
except HTTPException:
|
||||||
|
raise
|
||||||
|
except Exception:
|
||||||
|
_log.exception("POST /api/sessions/owner-backfill failed")
|
||||||
|
raise HTTPException(status_code=500, detail="Internal server error")
|
||||||
|
|
||||||
|
if stamped:
|
||||||
|
_log.info(
|
||||||
|
"owner-backfill: stamped %d legacy NULL-profile session row(s) with profile %r",
|
||||||
|
stamped,
|
||||||
|
stamp,
|
||||||
|
)
|
||||||
|
return {"ok": True, "stamped": stamped, "profile": stamp}
|
||||||
|
|
||||||
|
|
||||||
@manage_router.patch("/api/sessions/{session_id}")
|
@manage_router.patch("/api/sessions/{session_id}")
|
||||||
async def rename_session_endpoint(session_id: str, body: SessionRename):
|
async def rename_session_endpoint(session_id: str, body: SessionRename):
|
||||||
"""Update a session: rename, archive, hide, pin, and/or mark read/unread.
|
"""Update a session: rename, archive, hide, pin, and/or mark read/unread.
|
||||||
|
|||||||
@@ -9385,6 +9385,45 @@ class SessionDB(SessionSearchMixin, SessionSchemaMixin, SessionPortabilityMixin)
|
|||||||
|
|
||||||
return self._execute_write(_do) > 0
|
return self._execute_write(_do) > 0
|
||||||
|
|
||||||
|
def backfill_null_session_profiles(self, profile_name: str) -> int:
|
||||||
|
"""One-shot owner backfill for legacy pre-ownership session rows.
|
||||||
|
|
||||||
|
Sessions created before the durable-ownership work (#95407 lineage)
|
||||||
|
carry ``profile_name = NULL``. On single-backend installs that was
|
||||||
|
harmless, but once a Desktop registers a second connection the
|
||||||
|
fail-closed owner ladder (which is correct for new sessions) can no
|
||||||
|
longer route those rows anywhere — every pre-campaign session becomes
|
||||||
|
unresumable after upgrade (#94724, field report).
|
||||||
|
|
||||||
|
This store belongs to exactly one profile — the profile whose
|
||||||
|
``state.db`` this is — so stamping its own name onto rows that never
|
||||||
|
recorded one is a single-match backfill, not a guess. Rules mirror the
|
||||||
|
``create_session`` COALESCE contract:
|
||||||
|
|
||||||
|
* only ``NULL``/empty ``profile_name`` rows are touched — a non-NULL
|
||||||
|
owner is NEVER overwritten;
|
||||||
|
* idempotent and one-shot-per-row: a second run matches zero rows.
|
||||||
|
|
||||||
|
Returns the number of rows stamped (0 when nothing was legacy).
|
||||||
|
"""
|
||||||
|
stamp = (profile_name or "").strip()
|
||||||
|
if not stamp:
|
||||||
|
return 0
|
||||||
|
|
||||||
|
def _do(conn):
|
||||||
|
cursor = conn.execute(
|
||||||
|
"""UPDATE sessions
|
||||||
|
SET profile_name = ?
|
||||||
|
WHERE profile_name IS NULL OR TRIM(profile_name) = ''""",
|
||||||
|
(stamp,),
|
||||||
|
)
|
||||||
|
rowcount = cursor.rowcount
|
||||||
|
if rowcount is None or rowcount < 0:
|
||||||
|
rowcount = conn.execute("SELECT changes()").fetchone()[0]
|
||||||
|
return rowcount
|
||||||
|
|
||||||
|
return int(self._execute_write(_do) or 0)
|
||||||
|
|
||||||
def set_session_archived(self, session_id: str, archived: bool) -> bool:
|
def set_session_archived(self, session_id: str, archived: bool) -> bool:
|
||||||
"""Archive or unarchive a session.
|
"""Archive or unarchive a session.
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,141 @@
|
|||||||
|
"""Legacy NULL-profile session owner backfill (#94724).
|
||||||
|
|
||||||
|
Pre-#95407 session rows carry ``profile_name = NULL``. Once a Desktop has
|
||||||
|
registry topology (≥2 registered connections) the fail-closed owner ladder
|
||||||
|
can no longer route those rows, making every pre-campaign session
|
||||||
|
unresumable. POST /api/sessions/owner-backfill stamps each store's own
|
||||||
|
serving-profile identity onto its legacy rows — single-match by construction
|
||||||
|
(a profile's state.db belongs to exactly one profile), idempotent, and never
|
||||||
|
overwriting a non-NULL owner.
|
||||||
|
"""
|
||||||
|
|
||||||
|
import sqlite3
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
def client(monkeypatch, _isolate_hermes_home):
|
||||||
|
try:
|
||||||
|
from starlette.testclient import TestClient
|
||||||
|
except ImportError:
|
||||||
|
pytest.skip("fastapi/starlette not installed")
|
||||||
|
|
||||||
|
import hermes_state
|
||||||
|
from hermes_constants import get_hermes_home
|
||||||
|
from hermes_cli.web_server import app, _SESSION_HEADER_NAME, _SESSION_TOKEN
|
||||||
|
|
||||||
|
monkeypatch.setattr(hermes_state, "DEFAULT_DB_PATH", get_hermes_home() / "state.db")
|
||||||
|
|
||||||
|
client = TestClient(app)
|
||||||
|
client.headers[_SESSION_HEADER_NAME] = _SESSION_TOKEN
|
||||||
|
return client
|
||||||
|
|
||||||
|
|
||||||
|
def _seed(db_path, rows):
|
||||||
|
"""Insert bare session rows the way a pre-ownership install left them."""
|
||||||
|
from hermes_state import SessionDB
|
||||||
|
|
||||||
|
db = SessionDB(db_path=db_path)
|
||||||
|
try:
|
||||||
|
for session_id, profile_name in rows:
|
||||||
|
db.create_session(session_id, source="cli", profile_name=profile_name)
|
||||||
|
db.append_message(
|
||||||
|
session_id, role="user", content=f"hello from {session_id}"
|
||||||
|
)
|
||||||
|
# create_session backfills nothing here, but be explicit: force the
|
||||||
|
# legacy shape at the SQL level so the fixture cannot silently depend
|
||||||
|
# on create_session's own COALESCE behavior.
|
||||||
|
for session_id, profile_name in rows:
|
||||||
|
if profile_name is None:
|
||||||
|
db._conn.execute(
|
||||||
|
"UPDATE sessions SET profile_name = NULL WHERE id = ?",
|
||||||
|
(session_id,),
|
||||||
|
)
|
||||||
|
db._conn.commit()
|
||||||
|
finally:
|
||||||
|
db.close()
|
||||||
|
|
||||||
|
|
||||||
|
def _profiles(db_path):
|
||||||
|
conn = sqlite3.connect(str(db_path))
|
||||||
|
try:
|
||||||
|
return dict(conn.execute("SELECT id, profile_name FROM sessions").fetchall())
|
||||||
|
finally:
|
||||||
|
conn.close()
|
||||||
|
|
||||||
|
|
||||||
|
def test_backfill_stamps_only_null_rows_and_is_idempotent(client):
|
||||||
|
from hermes_constants import get_hermes_home
|
||||||
|
|
||||||
|
db_path = get_hermes_home() / "state.db"
|
||||||
|
_seed(
|
||||||
|
db_path,
|
||||||
|
[
|
||||||
|
("legacy-null-1", None),
|
||||||
|
("legacy-null-2", None),
|
||||||
|
("owned-other", "researcher"),
|
||||||
|
],
|
||||||
|
)
|
||||||
|
|
||||||
|
resp = client.post("/api/sessions/owner-backfill", json={})
|
||||||
|
assert resp.status_code == 200, resp.text
|
||||||
|
body = resp.json()
|
||||||
|
assert body["ok"] is True
|
||||||
|
# Exactly the two legacy rows were stamped; the count is reported so the
|
||||||
|
# caller can log it.
|
||||||
|
assert body["stamped"] == 2
|
||||||
|
assert body["profile"] == "default"
|
||||||
|
|
||||||
|
stamped = _profiles(db_path)
|
||||||
|
assert stamped["legacy-null-1"] == "default"
|
||||||
|
assert stamped["legacy-null-2"] == "default"
|
||||||
|
# Fail-closed contract: a non-NULL owner is NEVER overwritten, even when
|
||||||
|
# it names a different profile than the serving store.
|
||||||
|
assert stamped["owned-other"] == "researcher"
|
||||||
|
|
||||||
|
# One-shot-per-row: a second run finds nothing left to stamp and the rows
|
||||||
|
# are byte-identical.
|
||||||
|
resp2 = client.post("/api/sessions/owner-backfill", json={})
|
||||||
|
assert resp2.status_code == 200
|
||||||
|
assert resp2.json()["stamped"] == 0
|
||||||
|
assert _profiles(db_path) == stamped
|
||||||
|
|
||||||
|
|
||||||
|
def test_backfilled_rows_circulate_owned_on_the_list_endpoint(client):
|
||||||
|
"""After the backfill, the durable stamp (not just the per-response
|
||||||
|
serving-profile decoration) owns the rows: the raw DB column is non-NULL,
|
||||||
|
which is what survives into any other consumer of state.db."""
|
||||||
|
from hermes_constants import get_hermes_home
|
||||||
|
|
||||||
|
db_path = get_hermes_home() / "state.db"
|
||||||
|
_seed(db_path, [("legacy-null-3", None)])
|
||||||
|
|
||||||
|
assert _profiles(db_path)["legacy-null-3"] is None
|
||||||
|
|
||||||
|
resp = client.post("/api/sessions/owner-backfill", json={})
|
||||||
|
assert resp.status_code == 200
|
||||||
|
assert resp.json()["stamped"] == 1
|
||||||
|
|
||||||
|
listed = client.get("/api/sessions?limit=50&offset=0").json()["sessions"]
|
||||||
|
row = next(s for s in listed if s["id"] == "legacy-null-3")
|
||||||
|
assert row["profile"] == "default"
|
||||||
|
assert _profiles(db_path)["legacy-null-3"] == "default"
|
||||||
|
|
||||||
|
|
||||||
|
def test_backfill_treats_empty_string_profile_as_legacy(client):
|
||||||
|
"""TRIM('') rows are the same stranded class as NULL — stamp them too."""
|
||||||
|
from hermes_constants import get_hermes_home
|
||||||
|
|
||||||
|
db_path = get_hermes_home() / "state.db"
|
||||||
|
_seed(db_path, [("legacy-empty", None)])
|
||||||
|
|
||||||
|
conn = sqlite3.connect(str(db_path))
|
||||||
|
conn.execute("UPDATE sessions SET profile_name = ' ' WHERE id = 'legacy-empty'")
|
||||||
|
conn.commit()
|
||||||
|
conn.close()
|
||||||
|
|
||||||
|
resp = client.post("/api/sessions/owner-backfill", json={})
|
||||||
|
assert resp.status_code == 200
|
||||||
|
assert resp.json()["stamped"] == 1
|
||||||
|
assert _profiles(get_hermes_home() / "state.db")["legacy-empty"] == "default"
|
||||||
Reference in New Issue
Block a user