Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 35 additions & 3 deletions agent/title_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,38 @@ def generate_title(
return None


def _persist_session_title(session_db, session_id, title):
"""Persist a generated title, recovering from duplicate-title collisions.

set_session_title() raises ValueError when title would collide with another
session (the unique-title index). Rather than swallow it and leave the
session untitled (#50537), append a #N suffix via get_next_title_in_lineage()
when the store supports lineage dedup; otherwise re-raise so the caller can
decide. set_session_title() returning False means the session vanished
between generation and storage -> surface that as a RuntimeError instead of
silently dropping the title.

Returns the title actually persisted.
"""
def _set(t):
ok = session_db.set_session_title(session_id, t)
if ok is False:
raise RuntimeError(
f"session {session_id} not found when storing title"
)
return t
try:
return _set(title)
except ValueError:
next_title_fn = getattr(session_db, "get_next_title_in_lineage", None)
if next_title_fn is None:
raise
deduped = next_title_fn(title)
if not deduped or deduped == title:
raise
return _set(deduped)


def auto_title_session(
session_db,
session_id: str,
Expand Down Expand Up @@ -144,11 +176,11 @@ def auto_title_session(
return

try:
session_db.set_session_title(session_id, title)
logger.debug("Auto-generated session title: %s", title)
persisted = _persist_session_title(session_db, session_id, title)
logger.debug("Auto-generated session title: %s", persisted)
if title_callback is not None:
try:
title_callback(title)
title_callback(persisted)
except Exception:
logger.debug("Auto-title callback failed", exc_info=True)
except Exception as e:
Expand Down
76 changes: 68 additions & 8 deletions cron/scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,45 @@
logger = logging.getLogger(__name__)


def _set_cron_session_title(session_db, session_id, base_title):
"""Robustly title a finished cron session before it is closed.

Centralizes the title write so the cron finally block can guarantee a
non-blank, unique title is persisted before end_session()/close() tear
the connection down (issues #50535, #50536, #50537):

- #50535: never leaves the session blank. base_title already carries a
cron-id fallback for nameless jobs; this also guards a failed write.
- #50537: a duplicate title makes set_session_title raise ValueError (the
unique-title index). Recover by appending a #N suffix via
get_next_title_in_lineage() when supported, instead of swallowing the
error and ending up untitled. If lineage dedup is unavailable, raise.
- #50536: this runs synchronously in the cron finally block ahead of the
session close, so no in-flight title write can race the close.

Returns the title actually persisted, or None if nothing could be set.
"""
if not session_db or not session_id:
return None
title = (base_title or "").strip()
if not title:
return None
try:
session_db.set_session_title(session_id, title)
return title

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SessionDB.set_session_title() returns False when the session no longer exists (hermes_state.py:2947-2994). Check that result here (and for the deduplicated retry); otherwise this helper returns a truthy title that was never persisted, causing run_job() to skip its fallback.

except ValueError:
# Title collision against the unique-title index. Fall back to the
# next title in the lineage (base #2, base #3, ...) when supported.
next_title_fn = getattr(session_db, "get_next_title_in_lineage", None)
if next_title_fn is None:
raise
deduped = next_title_fn(title)
if not deduped or deduped == title:
raise
session_db.set_session_title(session_id, deduped)
return deduped


def _summarize_cron_failure_for_delivery(job: dict, error: str | None) -> str:
"""Return a compact one-line failure message for chat delivery.

Expand Down Expand Up @@ -2218,18 +2257,39 @@ def run_job(job: dict) -> tuple[bool, str, str, Optional[str]]:
for _var_name in _cron_delivery_vars:
_VAR_MAP[_var_name].set("")
if _session_db:
# Title the cron session from the job (name → short prompt → id) so
# sidebars/history show a meaningful label instead of the injected
# "[IMPORTANT: …]" hint that is the session's first message. Set here
# (not at create time) so the agent's own INSERT keeps model /
# system_prompt; this only UPDATEs the title column. The run-time
# suffix keeps it unique against the sessions.title index across runs.
# Title the cron session from the job (name -> id) and PERSIST it
# BEFORE end_session()/close() tear the connection down, so the
# close can never run over an in-flight title write (#50536). The
# run-time suffix keeps it unique against the sessions.title index
# across runs; _set_cron_session_title dedupes (#50537) and the
# except-fallback below guarantees a non-blank title (#50535).
try:
_title_base = " ".join(job_name.split())[:60].strip() or f"cron {job_id}"
_cron_title = f"{_title_base} · {_hermes_now().strftime('%b %d %H:%M')}"
_session_db.set_session_title(_cron_session_id, _cron_title)
if not _set_cron_session_title(_session_db, _cron_session_id, _cron_title):
# Helper returned None (blank base) -> use the id fallback.
_set_cron_session_title(
_session_db, _cron_session_id, f"cron {job_id}"
)
except (Exception, KeyboardInterrupt) as e:
logger.debug("Job '%s': failed to set cron session title: %s", job_id, e)
logger.debug(
"Job '%s': failed to set cron session title: %s", job_id, e
)
# Last-resort: never leave the session blank (#50535). Try the
# next free title in the lineage, then a bare id-stamped title.
for _fallback in (
getattr(_session_db, "get_next_title_in_lineage", lambda b: b)(
f"cron {job_id}"
),
f"cron {job_id} {_cron_session_id[-6:]}",
):
try:
if _set_cron_session_title(
_session_db, _cron_session_id, _fallback
):
break
except (Exception, KeyboardInterrupt):
continue
try:
_session_db.end_session(_cron_session_id, "cron_complete")
except (Exception, KeyboardInterrupt) as e:
Expand Down
44 changes: 44 additions & 0 deletions tests/agent/test_title_generator.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"""Tests for agent.title_generator — auto-generated session titles."""

import pytest
from unittest.mock import MagicMock, patch


Expand Down Expand Up @@ -273,3 +274,46 @@ def test_skips_if_no_response(self):

def test_skips_if_no_session_db(self):
maybe_auto_title(None, "sess-1", "hello", "response", []) # no db


class TestAutoTitleDuplicateHandling:
"""Duplicate auto-title handling and not-found hardening (#50537)."""

def test_dedupes_duplicate_title_via_lineage(self):
db = MagicMock()
db.get_session_title.return_value = None
db.set_session_title.side_effect = [ValueError("in use"), True]
db.get_next_title_in_lineage.return_value = "Debugging Import Error #2"
with patch(
"agent.title_generator.generate_title",
return_value="Debugging Import Error",
):
seen = []
auto_title_session(db, "sess-1", "hi", "hello", title_callback=seen.append)
db.get_next_title_in_lineage.assert_called_once_with("Debugging Import Error")
assert db.set_session_title.call_args_list[-1][0] == (
"sess-1",
"Debugging Import Error #2",
)
# callback fires with the actually-persisted (deduped) title
assert seen == ["Debugging Import Error #2"]

def test_swallows_value_error_without_lineage_support(self):
# No get_next_title_in_lineage -> ValueError propagates out of the
# persist helper but auto_title_session still swallows it (no crash).
db = MagicMock(spec=["get_session_title", "set_session_title"])
db.get_session_title.return_value = None
db.set_session_title.side_effect = ValueError("in use")
with patch(
"agent.title_generator.generate_title", return_value="Dup Title"
):
auto_title_session(db, "sess-1", "hi", "hello") # must not raise

def test_not_found_raises_runtime_error_internally(self):
# set_session_title returning False (session vanished) -> RuntimeError
# in the persist helper, swallowed by auto_title_session, no callback.
from agent.title_generator import _persist_session_title
db = MagicMock()
db.set_session_title.return_value = False
with pytest.raises(RuntimeError):
_persist_session_title(db, "missing", "Some Title")
40 changes: 40 additions & 0 deletions tests/cron/test_scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -3376,3 +3376,43 @@ def test_baileys_whatsapp_still_registered(self):
from cron.scheduler import _HOME_TARGET_ENV_VARS

assert _HOME_TARGET_ENV_VARS.get("whatsapp") == "WHATSAPP_HOME_CHANNEL"


class TestSetCronSessionTitle:
"""Robust cron session titling: #50535/#50536/#50537."""

def test_sets_title_when_no_collision(self):
from cron.scheduler import _set_cron_session_title
db = MagicMock()
db.set_session_title.return_value = True
out = _set_cron_session_title(db, "sess-1", "Nightly Synthesis")
assert out == "Nightly Synthesis"
db.set_session_title.assert_called_once_with("sess-1", "Nightly Synthesis")

def test_dedupes_on_duplicate_title(self):
# First write collides (ValueError); helper falls back to lineage #N.
from cron.scheduler import _set_cron_session_title
db = MagicMock()
db.set_session_title.side_effect = [ValueError("in use"), True]
db.get_next_title_in_lineage.return_value = "Nightly Synthesis #2"
out = _set_cron_session_title(db, "sess-1", "Nightly Synthesis")
assert out == "Nightly Synthesis #2"
db.get_next_title_in_lineage.assert_called_once_with("Nightly Synthesis")

def test_reraises_when_no_lineage_support(self):
from cron.scheduler import _set_cron_session_title
db = MagicMock(spec=["set_session_title"])
db.set_session_title.side_effect = ValueError("in use")
with pytest.raises(ValueError):
_set_cron_session_title(db, "sess-1", "Dup")

def test_returns_none_for_blank_base(self):
from cron.scheduler import _set_cron_session_title
db = MagicMock()
assert _set_cron_session_title(db, "sess-1", " ") is None
db.set_session_title.assert_not_called()

def test_returns_none_without_db_or_session(self):
from cron.scheduler import _set_cron_session_title
assert _set_cron_session_title(None, "sess-1", "X") is None
assert _set_cron_session_title(MagicMock(), "", "X") is None