fix(sessions): close the session_switch legacy gap and fence the resume walker at reset boundaries
Follow-ups to the salvaged #84009 commits: - Add 'session_switch' to _RESET_END_REASONS: a reset continuation's parent can be promoted to session_switch (resume the reset parent, then switch away), which permanently hid pre-marker legacy children — reopen-time stamping cannot rescue them because the parent is being ended, not reopened. Probe-verified before/after. - Share the legacy reset-child heuristic via _legacy_reset_child_sql() so _RESET_CHILD_SQL and reopen_session()'s stamping UPDATE cannot drift, and derive find_latest_gateway_session_for_peer's two recovery fence literals from _RESET_END_REASONS_SQL (was a third hand-written copy of the same set). - Exclude reset children (marker + legacy shape) from the resolve_resume_session_id forward walker: resuming a reset parent could redirect into the post-reset conversation — the exact context the user reset away. Regression tests cover both shapes plus the walker's original compression-tip behavior; mutation-checked.
This commit is contained in:
+22
-21
@@ -55,7 +55,9 @@ from hermes_state_common import ( # noqa: F401 (re-exported for back-compat)
|
||||
_LISTABLE_CHILD_SQL,
|
||||
_PREVIEW_RAW_SELECT,
|
||||
_RESET_END_REASONS,
|
||||
_RESET_END_REASONS_SQL,
|
||||
_ephemeral_child_sql,
|
||||
_legacy_reset_child_sql,
|
||||
_shape_preview,
|
||||
_sql_session_last_active,
|
||||
_sql_session_last_active_by_id,
|
||||
@@ -4587,7 +4589,7 @@ class SessionDB(SessionSearchMixin, SessionSchemaMixin, SessionPortabilityMixin)
|
||||
return None
|
||||
with self._lock:
|
||||
row = self._conn.execute(
|
||||
"""
|
||||
f"""
|
||||
SELECT s.*,
|
||||
COALESCE(sp.prompt, s.system_prompt)
|
||||
AS _system_prompt_resolved,
|
||||
@@ -4604,9 +4606,7 @@ class SessionDB(SessionSearchMixin, SessionSchemaMixin, SessionPortabilityMixin)
|
||||
WHERE b.session_key = s.session_key
|
||||
AND b.source = s.source
|
||||
AND b.ended_at IS NOT NULL
|
||||
AND b.end_reason IN ('session_reset', 'session_switch',
|
||||
'idle', 'daily', 'suspended',
|
||||
'resume_pending_expired')
|
||||
AND b.end_reason IN ({_RESET_END_REASONS_SQL})
|
||||
AND b.ended_at
|
||||
> COALESCE(s.last_activity_at, s.started_at)
|
||||
)
|
||||
@@ -4625,7 +4625,7 @@ class SessionDB(SessionSearchMixin, SessionSchemaMixin, SessionPortabilityMixin)
|
||||
if chat_id is None or chat_type is None:
|
||||
return None
|
||||
row = self._conn.execute(
|
||||
"""
|
||||
f"""
|
||||
SELECT s.*,
|
||||
COALESCE(sp.prompt, s.system_prompt)
|
||||
AS _system_prompt_resolved,
|
||||
@@ -4651,9 +4651,7 @@ class SessionDB(SessionSearchMixin, SessionSchemaMixin, SessionPortabilityMixin)
|
||||
AND COALESCE(b.chat_type, '') = COALESCE(s.chat_type, '')
|
||||
AND COALESCE(b.thread_id, '') = COALESCE(s.thread_id, '')
|
||||
AND b.ended_at IS NOT NULL
|
||||
AND b.end_reason IN ('session_reset', 'session_switch',
|
||||
'idle', 'daily', 'suspended',
|
||||
'resume_pending_expired')
|
||||
AND b.end_reason IN ({_RESET_END_REASONS_SQL})
|
||||
AND b.ended_at
|
||||
> COALESCE(s.last_activity_at, s.started_at)
|
||||
)
|
||||
@@ -5163,6 +5161,9 @@ class SessionDB(SessionSearchMixin, SessionSchemaMixin, SessionPortabilityMixin)
|
||||
"""
|
||||
def _do(conn):
|
||||
placeholders = ",".join("?" for _ in _RESET_END_REASONS)
|
||||
# WHERE shape shared with _RESET_CHILD_SQL's fallback arm via
|
||||
# _legacy_reset_child_sql so the stamping and the listing
|
||||
# predicate cannot drift.
|
||||
conn.execute(
|
||||
"UPDATE sessions AS child SET model_config = json_set("
|
||||
"COALESCE(child.model_config, '{}'), '$._reset_from', "
|
||||
@@ -5170,12 +5171,7 @@ class SessionDB(SessionSearchMixin, SessionSchemaMixin, SessionPortabilityMixin)
|
||||
"WHERE child.parent_session_id = ? "
|
||||
"AND json_extract(COALESCE(child.model_config, '{}'), "
|
||||
" '$._reset_from') IS NULL "
|
||||
"AND child.session_key IS NOT NULL "
|
||||
"AND child.session_key != '' "
|
||||
"AND EXISTS (SELECT 1 FROM sessions parent "
|
||||
" WHERE parent.id = child.parent_session_id "
|
||||
f" AND parent.end_reason IN ({placeholders}) "
|
||||
" AND parent.session_key = child.session_key)",
|
||||
f"AND {_legacy_reset_child_sql('child', placeholders)}",
|
||||
(session_id, *_RESET_END_REASONS),
|
||||
)
|
||||
conn.execute(
|
||||
@@ -9017,18 +9013,23 @@ class SessionDB(SessionSearchMixin, SessionSchemaMixin, SessionPortabilityMixin)
|
||||
|
||||
# Walk to the most-recently-started child — but skip explicit
|
||||
# branch (`_branched_from`), delegate/subagent (`_delegate_from`),
|
||||
# and tool children. They also carry a ``parent_session_id`` yet
|
||||
# reset-continuation (`_reset_from` or the legacy same-key
|
||||
# heuristic — a post-reset conversation must never be reached
|
||||
# by resuming the parent the user reset away), and tool
|
||||
# children. They also carry a ``parent_session_id`` yet
|
||||
# are NOT compression continuations; following them would hijack
|
||||
# the resume target to an unrelated session (e.g. a subagent
|
||||
# run). This mirrors the child-exclusion in ``get_compression_tip``.
|
||||
try:
|
||||
child_row = self._conn.execute(
|
||||
"SELECT id FROM sessions "
|
||||
"WHERE parent_session_id = ? "
|
||||
" AND json_extract(COALESCE(model_config, '{}'), '$._branched_from') IS NULL "
|
||||
" AND json_extract(COALESCE(model_config, '{}'), '$._delegate_from') IS NULL "
|
||||
" AND COALESCE(source, '') != 'tool' "
|
||||
"ORDER BY started_at DESC, id DESC LIMIT 1",
|
||||
"SELECT id FROM sessions AS child "
|
||||
"WHERE child.parent_session_id = ? "
|
||||
" AND json_extract(COALESCE(child.model_config, '{}'), '$._branched_from') IS NULL "
|
||||
" AND json_extract(COALESCE(child.model_config, '{}'), '$._delegate_from') IS NULL "
|
||||
" AND json_extract(COALESCE(child.model_config, '{}'), '$._reset_from') IS NULL "
|
||||
f" AND NOT {_legacy_reset_child_sql('child', _RESET_END_REASONS_SQL)} "
|
||||
" AND COALESCE(child.source, '') != 'tool' "
|
||||
"ORDER BY child.started_at DESC, child.id DESC LIMIT 1",
|
||||
(current,),
|
||||
).fetchone()
|
||||
except Exception:
|
||||
|
||||
+27
-6
@@ -100,6 +100,13 @@ _COMPRESSION_CHILD_SQL = (
|
||||
|
||||
_RESET_END_REASONS = (
|
||||
"session_reset",
|
||||
# switch_session() never creates a child row, but pre-marker DBs can hold
|
||||
# legacy reset children whose parent later ended with 'session_switch'
|
||||
# (resumed then switched away before reopen-time stamping existed). Also
|
||||
# keeps this set identical to the recovery fence in
|
||||
# find_latest_gateway_session_for_peer, which interpolates
|
||||
# _RESET_END_REASONS_SQL so the two cannot drift.
|
||||
"session_switch",
|
||||
"idle",
|
||||
"daily",
|
||||
"suspended",
|
||||
@@ -108,6 +115,25 @@ _RESET_END_REASONS = (
|
||||
_RESET_END_REASONS_SQL = ", ".join(f"'{reason}'" for reason in _RESET_END_REASONS)
|
||||
|
||||
|
||||
def _legacy_reset_child_sql(alias: str, reasons_sql: str) -> str:
|
||||
"""Pre-marker reset-continuation heuristic.
|
||||
|
||||
A child is a legacy reset continuation when it rides its parent's exact
|
||||
non-empty routing key and the parent ended at a reset boundary. Shared by
|
||||
the listing predicate (``_RESET_CHILD_SQL``) and ``reopen_session()``'s
|
||||
marker-stamping UPDATE so the two sites cannot drift; ``reasons_sql`` is
|
||||
either the literal ``_RESET_END_REASONS_SQL`` or a bound-placeholder list.
|
||||
"""
|
||||
return (
|
||||
f"EXISTS (SELECT 1 FROM sessions p"
|
||||
f" WHERE p.id = {alias}.parent_session_id"
|
||||
f" AND p.end_reason IN ({reasons_sql})"
|
||||
f" AND {alias}.session_key IS NOT NULL"
|
||||
f" AND {alias}.session_key != ''"
|
||||
f" AND {alias}.session_key = p.session_key)"
|
||||
)
|
||||
|
||||
|
||||
# A reset starts a separate user-visible conversation even though gateway rows
|
||||
# retain parent_session_id for durable lineage. New rows carry the stable
|
||||
# marker; the same-key fallback recovers rows written before the marker existed.
|
||||
@@ -115,12 +141,7 @@ _RESET_END_REASONS_SQL = ", ".join(f"'{reason}'" for reason in _RESET_END_REASON
|
||||
# out even when their parent is later reset.
|
||||
_RESET_CHILD_SQL = (
|
||||
"json_extract(COALESCE({a}.model_config, '{{}}'), '$._reset_from') IS NOT NULL"
|
||||
" OR EXISTS (SELECT 1 FROM sessions p"
|
||||
" WHERE p.id = {a}.parent_session_id"
|
||||
f" AND p.end_reason IN ({_RESET_END_REASONS_SQL})"
|
||||
" AND {a}.session_key IS NOT NULL"
|
||||
" AND {a}.session_key != ''"
|
||||
" AND {a}.session_key = p.session_key)"
|
||||
" OR " + _legacy_reset_child_sql("{a}", _RESET_END_REASONS_SQL)
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -1854,6 +1854,7 @@ class TestListSessionsRich:
|
||||
"end_reason",
|
||||
[
|
||||
"session_reset",
|
||||
"session_switch",
|
||||
"idle",
|
||||
"daily",
|
||||
"suspended",
|
||||
@@ -1904,6 +1905,44 @@ class TestListSessionsRich:
|
||||
assert "unrelated_child" not in listed
|
||||
assert db.session_count(exclude_children=True) == 1
|
||||
|
||||
def test_resume_walker_does_not_cross_reset_boundary(self, db):
|
||||
"""resolve_resume_session_id must not redirect a reset parent's resume
|
||||
into the post-reset conversation — that would restore the exact
|
||||
context the user reset away. Covers both the durable marker and the
|
||||
legacy markerless shape."""
|
||||
lane_key = "agent:main:telegram:dm:lane"
|
||||
# Marker shape (rows written by current gateway code).
|
||||
db.create_session("walk_parent", "telegram", session_key=lane_key)
|
||||
db.append_message("walk_parent", "user", "before reset")
|
||||
db.end_session("walk_parent", "session_reset")
|
||||
db.create_session(
|
||||
"walk_child",
|
||||
"telegram",
|
||||
session_key=lane_key,
|
||||
parent_session_id="walk_parent",
|
||||
model_config={"_reset_from": "walk_parent"},
|
||||
)
|
||||
db.append_message("walk_child", "user", "after reset")
|
||||
assert db.resolve_resume_session_id("walk_parent") == "walk_parent"
|
||||
|
||||
# Legacy markerless shape (pre-marker on-disk rows).
|
||||
lane2 = "agent:main:telegram:dm:lane2"
|
||||
db.create_session("legacy_parent", "telegram", session_key=lane2)
|
||||
db.append_message("legacy_parent", "user", "before reset")
|
||||
db.end_session("legacy_parent", "session_reset")
|
||||
db.create_session(
|
||||
"legacy_child",
|
||||
"telegram",
|
||||
session_key=lane2,
|
||||
parent_session_id="legacy_parent",
|
||||
)
|
||||
db.append_message("legacy_child", "user", "after reset")
|
||||
assert db.resolve_resume_session_id("legacy_parent") == "legacy_parent"
|
||||
|
||||
# Compression-tip following (the walker's original purpose) is pinned by
|
||||
# tests/hermes_state/test_resolve_resume_session_id.py
|
||||
# ::test_follows_compression_tip_when_parent_retains_messages.
|
||||
|
||||
def test_session_key_predicate_can_use_session_key_index(self, db):
|
||||
plan = db._conn.execute(
|
||||
"EXPLAIN QUERY PLAN "
|
||||
|
||||
Reference in New Issue
Block a user