fix(cron): robust session title generation (#50535, #50536, #50537)

agent/title_generator.py

@@ -138,6 +138,53 @@ def generate_title(
         return None
 
 
+def _persist_session_title(session_db, session_id, title):
+    """Persist a generated title, recovering from duplicate-title collisions.
+
+    The write goes through ``set_auto_title_if_empty`` (predicate + write in
+    one transaction) so a manual ``/title`` set while LLM generation was in
+    flight is never overwritten — a plain ``set_session_title`` fallback keeps
+    older stores working. ``set_session_title`` raises ValueError when the
+    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.
+
+    Returns the title actually persisted, or None when a concurrent manual
+    title won the race (nothing was written).
+    """
+    atomic_fn = getattr(session_db, "set_auto_title_if_empty", None)
+
+    def _set(t):
+        if atomic_fn is not None:
+            if not atomic_fn(session_id, t):
+                # Predicate failed: a title appeared while generation was in
+                # flight (manual /title wins), or the session vanished.
+                logger.debug(
+                    "Skipping auto-generated session title because a title "
+                    "was set while generation was in flight"
+                )
+                return None
+            return 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,
@@ -237,15 +284,13 @@ def _auto_title_session(
         return
 
     try:
-        if not session_db.set_auto_title_if_empty(session_id, title):
-            logger.debug(
-                "Skipping auto-generated session title because a title was set while generation was in flight"
-            )
+        persisted = _persist_session_title(session_db, session_id, title)
+        if persisted is None:
             return
-        logger.debug("Auto-generated session title: %s", 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:

cron/scheduler.py

@@ -48,6 +48,45 @@ from hermes_time import now as _hermes_now
 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
+    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.
 
@@ -3500,18 +3539,39 @@ def run_job(
         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:

tests/agent/test_title_generator.py

@@ -1,5 +1,6 @@
 """Tests for agent.title_generator — auto-generated session titles."""
 
+import pytest
 from unittest.mock import MagicMock, patch
 
 
@@ -437,3 +438,46 @@ class TestMaybeAutoTitle:
 
     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")

tests/cron/test_scheduler.py

@@ -4942,3 +4942,43 @@ class TestMultiTargetDeliveryContinuesOnFailure:
         assert "a@example.com" in result
         assert "b@example.com" in result
         assert mock_pool.submit.call_count == 2
+
+
+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