diff --git a/hermes_state.py b/hermes_state.py index dd4f3e25ec..800a7828e1 100644 --- a/hermes_state.py +++ b/hermes_state.py @@ -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: diff --git a/hermes_state_common.py b/hermes_state_common.py index bb8539e1bd..a6f29dbe75 100644 --- a/hermes_state_common.py +++ b/hermes_state_common.py @@ -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) ) diff --git a/tests/test_hermes_state.py b/tests/test_hermes_state.py index fa25b08e2d..8927560493 100644 --- a/tests/test_hermes_state.py +++ b/tests/test_hermes_state.py @@ -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 "