From e1ec41b32cdbb720f73f1525d64f8b8a53b3b98b Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Sat, 13 Jun 2026 16:47:57 -0400 Subject: [PATCH] fix(session): reload latest imported session before title save (#3987) --- api/routes.py | 131 ++++++++++++------ .../test_issue3987_imported_session_titles.py | 82 ++++++++++- 2 files changed, 169 insertions(+), 44 deletions(-) diff --git a/api/routes.py b/api/routes.py index 42f79c2f..e4ab9838 100644 --- a/api/routes.py +++ b/api/routes.py @@ -79,55 +79,106 @@ def _sync_session_title_to_insights(session) -> None: logger.debug("Failed to update session title in state.db", exc_info=True) -def _persist_generated_session_title(session, next_title: str, *, event_reason: str) -> str: +def _persist_generated_session_title( + session, + next_title: str, + *, + event_reason: str, + require_default_title: bool = False, +) -> str: normalized_title = str(next_title or "").strip()[:80] or "Untitled" - with _get_session_agent_lock(session.session_id): - session.title = normalized_title + sid = str(getattr(session, "session_id", "") or "") + with _get_session_agent_lock(sid): + with LOCK: + latest = SESSIONS.get(sid) + if latest is not None and str(getattr(latest, "session_id", "") or "") != sid: + SESSIONS.pop(sid, None) + latest = None + elif latest is not None: + SESSIONS.move_to_end(sid) + if latest is None: + latest = Session.load(sid) + if latest is None: + raise KeyError(sid) + latest = _ensure_full_session_before_mutation(sid, latest) + if getattr(latest, "read_only", False): + raise PermissionError(f"Session {sid} is read-only") + if require_default_title: + latest_meta = { + "title": getattr(latest, "title", None), + "source_tag": getattr(latest, "source_tag", None), + "raw_source": getattr(latest, "raw_source", None), + "session_source": getattr(latest, "session_source", None), + "source_label": getattr(latest, "source_label", None), + } + if not _looks_like_default_cli_title(latest_meta): + return latest.title + latest.title = normalized_title from api.session_ops import mark_session_title_generated # mark_session_title_generated sets s.llm_title_generated = True and clears manual_title. - mark_session_title_generated(session) - session.save(touch_updated_at=False) - _sync_session_title_to_insights(session) - _publish_session_list_changed(event_reason, profile=getattr(session, "profile", None)) - return session.title + mark_session_title_generated(latest) + latest.save(touch_updated_at=False) + with LOCK: + SESSIONS[sid] = latest + SESSIONS.move_to_end(sid) + while len(SESSIONS) > SESSIONS_MAX: + SESSIONS.popitem(last=False) + session.title = latest.title + session.llm_title_generated = latest.llm_title_generated + session.manual_title = latest.manual_title + _sync_session_title_to_insights(latest) + _publish_session_list_changed(event_reason, profile=getattr(latest, "profile", None)) + return latest.title def _queue_generated_title_for_imported_session(session, cli_meta: dict | None) -> None: - cli_meta = dict(cli_meta or {}) - if not session or cli_meta.get("read_only") or not _looks_like_default_cli_title(cli_meta): - return - sid = str(getattr(session, "session_id", "") or "") - if not sid: - return + try: + cli_meta = dict(cli_meta or {}) + if not session or cli_meta.get("read_only") or not _looks_like_default_cli_title(cli_meta): + return + sid = str(getattr(session, "session_id", "") or "") + if not sid: + return - def _run() -> None: - try: - current = Session.load(sid) - if not current: - return - current = _ensure_full_session_before_mutation(sid, current) - if getattr(current, "read_only", False): - return - current_meta = { - "title": getattr(current, "title", None), - "source_tag": getattr(current, "source_tag", None), - "raw_source": getattr(current, "raw_source", None), - "session_source": getattr(current, "session_source", None), - "source_label": getattr(current, "source_label", None), - } - if not _looks_like_default_cli_title(current_meta): - return - next_title, _reason, _raw_preview = generate_session_title_for_session(current) - normalized_current = str(getattr(current, "title", "") or "").strip() - normalized_next = str(next_title or "").strip() - if not normalized_next or normalized_next == normalized_current: - return - _persist_generated_session_title(current, normalized_next, event_reason="session_title_regenerate") - except Exception: - logger.debug("Failed to generate imported session title for %s", sid, exc_info=True) + def _run() -> None: + try: + current = Session.load(sid) + if not current: + return + current = _ensure_full_session_before_mutation(sid, current) + if getattr(current, "read_only", False): + return + current_meta = { + "title": getattr(current, "title", None), + "source_tag": getattr(current, "source_tag", None), + "raw_source": getattr(current, "raw_source", None), + "session_source": getattr(current, "session_source", None), + "source_label": getattr(current, "source_label", None), + } + if not _looks_like_default_cli_title(current_meta): + return + next_title, _reason, _raw_preview = generate_session_title_for_session(current) + normalized_current = str(getattr(current, "title", "") or "").strip() + normalized_next = str(next_title or "").strip() + if not normalized_next or normalized_next == normalized_current: + return + _persist_generated_session_title( + current, + normalized_next, + event_reason="session_title_regenerate", + require_default_title=True, + ) + except Exception: + logger.debug("Failed to generate imported session title for %s", sid, exc_info=True) - threading.Thread(target=_run, daemon=True, name=f"imported-title-{sid}").start() + threading.Thread(target=_run, daemon=True, name=f"imported-title-{sid}").start() + except Exception: + logger.debug( + "Failed to queue imported session title generation for %s", + getattr(session, "session_id", None), + exc_info=True, + ) # ── Cron run tracking ──────────────────────────────────────────────────────── diff --git a/tests/test_issue3987_imported_session_titles.py b/tests/test_issue3987_imported_session_titles.py index 027823f7..5b77189b 100644 --- a/tests/test_issue3987_imported_session_titles.py +++ b/tests/test_issue3987_imported_session_titles.py @@ -4,8 +4,10 @@ from __future__ import annotations import io import json +from collections import OrderedDict from pathlib import Path +import api.models as models import api.routes as routes @@ -54,7 +56,7 @@ def test_import_cli_queue_helper_is_guarded_and_runs_in_background(): assert "not _looks_like_default_cli_title(cli_meta)" in block assert "if not _looks_like_default_cli_title(current_meta):" in block assert "generate_session_title_for_session(current)" in block - assert "_persist_generated_session_title(current, normalized_next, event_reason=\"session_title_regenerate\")" in block + assert "require_default_title=True" in block assert "threading.Thread(target=_run, daemon=True" in block @@ -85,12 +87,18 @@ def test_import_cli_queue_helper_generates_title_once_for_placeholder_session(mo monkeypatch.setattr(routes.Session, "load", classmethod(lambda _cls, sid: current if sid == current.session_id else None)) monkeypatch.setattr(routes, "_ensure_full_session_before_mutation", lambda sid, session: session) monkeypatch.setattr(routes, "generate_session_title_for_session", lambda session: (generated.append(session.session_id) or "Better imported title", "llm", "raw")) - monkeypatch.setattr(routes, "_persist_generated_session_title", lambda session, title, *, event_reason: persisted.append((session.session_id, title, event_reason))) + monkeypatch.setattr( + routes, + "_persist_generated_session_title", + lambda session, title, *, event_reason, require_default_title=False: persisted.append( + (session.session_id, title, event_reason, require_default_title) + ), + ) routes._queue_generated_title_for_imported_session(current, {"title": "CLI Session", "source_tag": "cli"}) assert generated == [current.session_id] - assert persisted == [(current.session_id, "Better imported title", "session_title_regenerate")] + assert persisted == [(current.session_id, "Better imported title", "session_title_regenerate", True)] def test_import_cli_queue_helper_skips_sessions_that_already_have_real_titles(monkeypatch): @@ -124,7 +132,13 @@ def test_import_cli_queue_helper_skips_sessions_that_already_have_real_titles(mo "generate_session_title_for_session", lambda session: generated.append(session.session_id) or ("Unexpected generated title", "llm", "raw"), ) - monkeypatch.setattr(routes, "_persist_generated_session_title", lambda session, title, *, event_reason: persisted.append((session.session_id, title, event_reason))) + monkeypatch.setattr( + routes, + "_persist_generated_session_title", + lambda session, title, *, event_reason, require_default_title=False: persisted.append( + (session.session_id, title, event_reason, require_default_title) + ), + ) routes._queue_generated_title_for_imported_session( current, @@ -135,6 +149,66 @@ def test_import_cli_queue_helper_skips_sessions_that_already_have_real_titles(mo assert persisted == [] +def test_generated_title_persist_reloads_latest_session_before_saving(tmp_path, monkeypatch): + session_dir = tmp_path / "sessions" + session_dir.mkdir() + cache = OrderedDict() + + monkeypatch.setattr(models, "SESSION_DIR", session_dir) + monkeypatch.setattr(models, "SESSIONS", cache, raising=False) + monkeypatch.setattr(routes, "SESSIONS", cache, raising=False) + monkeypatch.setattr(routes, "_sync_session_title_to_insights", lambda session: None) + monkeypatch.setattr(routes, "_publish_session_list_changed", lambda *args, **kwargs: None) + + stale = models.Session( + session_id="cli_import_race", + title="CLI Session", + workspace=".", + model="test-model", + messages=[{"role": "user", "content": "first"}], + source_tag="cli", + raw_source="cli", + session_source="external_agent", + source_label="CLI", + ) + stale.save(skip_index=True) + stale_snapshot = models.Session.load(stale.session_id) + + latest = models.Session( + session_id=stale.session_id, + title="CLI Session", + workspace=".", + model="test-model", + messages=[ + {"role": "user", "content": "first"}, + {"role": "assistant", "content": "reply"}, + {"role": "user", "content": "second"}, + ], + source_tag="cli", + raw_source="cli", + session_source="external_agent", + source_label="CLI", + ) + latest.save(skip_index=True) + cache[latest.session_id] = latest + + saved_title = routes._persist_generated_session_title( + stale_snapshot, + "Better imported title", + event_reason="session_title_regenerate", + require_default_title=True, + ) + + reloaded = models.Session.load(stale.session_id) + assert saved_title == "Better imported title" + assert reloaded.title == "Better imported title" + assert len(reloaded.messages) == 3 + assert reloaded.llm_title_generated is True + assert reloaded.manual_title is False + assert len(cache[stale.session_id].messages) == 3 + assert stale_snapshot.title == "Better imported title" + + def test_regenerate_endpoint_only_blocks_read_only_imported_sessions(): endpoint_idx = ROUTES_PY.index('"/api/session/title/regenerate"') next_endpoint_idx = ROUTES_PY.index('"/api/personality/set"', endpoint_idx)