From 0b1a4cc17618335decd87b7c9f44af861988a702 Mon Sep 17 00:00:00 2001 From: Nathan Esquenazi Date: Mon, 27 Apr 2026 16:31:48 -0700 Subject: [PATCH 1/2] fix(models): defer first save() until session has real state (#1171 follow-up) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #1182 stopped surfacing empty Untitled sessions in the sidebar but left the underlying disk-pile-up: every `/api/session/new` call still wrote a JSON file via the eager `s.save()` at the end of `new_session()`. Reload, click New Conversation, complete onboarding without sending a message — each one orphaned a file under `~/.hermes/webui/sessions/`. Fix: drop the eager save. A session lives in the in-memory `SESSIONS` dict from creation; the first disk write happens at the natural "this is now a real session" moment: 1. `_handle_chat_start` sets `pending_user_message` and calls `s.save()` before launching the streaming thread (api/routes.py:2806). This is the path for the very first user message. 2. btw and background-agent paths populate `title` + `messages` and call `.save()` themselves immediately after `new_session()` (api/routes.py:2710, 2752). Those continue to persist as before. The two tests that broke required only the targeted update they deserved: - `test_api_session_is_side_effect_free_for_stale_models` was reading the on-disk file straight after POST; updated to materialise the file from the API response when it isn't present, since the test's intent is verifying that a SUBSEQUENT GET doesn't rewrite the file. - The 7 new tests in `test_empty_session_no_disk_write.py` lock the new contract: - new_session() does not write to disk - the session lives in SESSIONS and is findable by get_session - an unsaved Untitled+0-msg session does not appear in all_sessions() output (the #1171 filter still applies) - save() writes when first invoked (post-message-append) - btw / background pattern still persists - five news in a row produce zero JSON files Crash-safety trade-off: if the process exits between create and first message, the unsaved session is lost. There were no messages to lose, so this is the correct semantics — only conversations that received real input get persisted. Builds on #1171 (server filter + button guard) and #1182 (boot early-exit, full-scan-fallback consistency). Local full suite: 2637 passed, 47 skipped, 1 unrelated pre-existing failure on macOS (test_sprint3 system-paths quirk). Co-Authored-By: Claude Opus 4.7 (1M context) --- api/models.py | 17 ++- tests/test_empty_session_no_disk_write.py | 130 ++++++++++++++++++++++ tests/test_provider_mismatch.py | 10 ++ 3 files changed, 155 insertions(+), 2 deletions(-) create mode 100644 tests/test_empty_session_no_disk_write.py diff --git a/api/models.py b/api/models.py index a8aa015e93a..4a01db51342 100644 --- a/api/models.py +++ b/api/models.py @@ -484,7 +484,21 @@ def get_session(sid, metadata_only=False): raise KeyError(sid) def new_session(workspace=None, model=None, profile=None): - """Create a new in-memory session and persist it. + """Create a new in-memory session. + + The session lives in the SESSIONS dict only — no disk write happens until + the first message is appended (#1171 follow-up). This avoids the + "ghost Untitled session on disk" pile-up that occurred when users clicked + New Conversation, reloaded the page, or completed onboarding without ever + sending a message. Subsequent code paths that populate state immediately + (btw / background agent at api/routes.py) call ``s.save()`` themselves + after setting title/messages, and ``_handle_chat_start`` saves the + session as soon as the user actually sends a message — both are the + natural first-write moments for a real session. + + Crash-safety: if the process exits between session creation and first + message, the session is lost. Since it had no messages, there is + nothing to lose. *profile* — when supplied by the caller (e.g. from the request body sent by the active browser tab), it is used directly so that concurrent clients @@ -510,7 +524,6 @@ def new_session(workspace=None, model=None, profile=None): SESSIONS.move_to_end(s.session_id) while len(SESSIONS) > SESSIONS_MAX: SESSIONS.popitem(last=False) - s.save() return s def _hide_from_default_sidebar(session: dict) -> bool: diff --git a/tests/test_empty_session_no_disk_write.py b/tests/test_empty_session_no_disk_write.py new file mode 100644 index 00000000000..9d73475f9c3 --- /dev/null +++ b/tests/test_empty_session_no_disk_write.py @@ -0,0 +1,130 @@ +""" +Regression tests for the "no disk write for empty sessions" follow-up to #1171. + +Lifecycle contract: + 1. ``new_session()`` adds the session to the in-memory ``SESSIONS`` dict but + does NOT write a JSON file to disk. + 2. The first ``s.save()`` happens when the session has real state to persist + (a user message via ``/api/chat/start``, or a populated title/messages + for btw / background agents). + 3. ``get_session(sid)`` is unchanged: it checks ``SESSIONS`` first, so an + unsaved session is still findable by ID for the brief window between + create and first message. + 4. ``all_sessions()`` already filters Untitled + 0-message sessions (#1171), + so an unsaved in-memory session does not surface in the sidebar even + though it lives in the SESSIONS dict. + +Crash-safety: if the process exits between create and first message, the +session is lost. There were no messages to lose, so this is an explicit +trade-off documented in ``new_session``'s docstring. +""" +import json +import time + +import pytest + +import api.models as models +from api.models import ( + SESSIONS, + Session, + all_sessions, + get_session, + new_session, +) + + +@pytest.fixture(autouse=True) +def _isolate(tmp_path, monkeypatch): + """Redirect SESSION_DIR and SESSION_INDEX_FILE to a fresh tmp dir.""" + session_dir = tmp_path / "sessions" + session_dir.mkdir() + index_file = session_dir / "_index.json" + monkeypatch.setattr(models, "SESSION_DIR", session_dir) + monkeypatch.setattr(models, "SESSION_INDEX_FILE", index_file) + SESSIONS.clear() + yield session_dir + SESSIONS.clear() + + +# ── 1. new_session does not write to disk ─────────────────────────────────── + + +def test_new_session_does_not_write_to_disk(_isolate): + s = new_session() + assert not s.path.exists(), ( + "new_session() must not eagerly persist an empty session — disk write " + "is deferred until the first message is appended (#1171 follow-up)" + ) + + +def test_new_session_lives_in_memory(_isolate): + s = new_session() + assert s.session_id in SESSIONS + assert SESSIONS[s.session_id] is s + + +def test_get_session_finds_unsaved_session_by_id(_isolate): + """The brief window between create and first message must still allow + /api/chat/start to look up the session by its returned session_id.""" + s = new_session() + found = get_session(s.session_id) + assert found is s, ( + "get_session must return the in-memory unsaved session — _handle_chat_start " + "depends on this for the very first message in a fresh session." + ) + + +# ── 2. unsaved sessions never surface in the sidebar ───────────────────────── + + +def test_unsaved_empty_session_hidden_from_sidebar(_isolate): + """all_sessions filters Untitled+0-message regardless of save state (#1171).""" + s = new_session() + ids = {row["session_id"] for row in all_sessions()} + assert s.session_id not in ids, ( + "An unsaved empty Untitled session must not appear in /api/sessions" + ) + + +# ── 3. save() materialises the file when state is real ───────────────────── + + +def test_save_writes_to_disk_when_first_invoked(_isolate): + """The first save() (typically from _handle_chat_start after appending a + user message) creates the JSON file.""" + s = new_session() + assert not s.path.exists() + s.messages.append({"role": "user", "content": "hello"}) + s.save() + assert s.path.exists(), "save() must create the file once it's called" + content = json.loads(s.path.read_text(encoding="utf-8")) + assert content["session_id"] == s.session_id + assert content["messages"] and content["messages"][0]["role"] == "user" + + +def test_btw_background_pattern_still_persists(_isolate): + """btw / background agents at api/routes.py call save() right after + populating title/messages — that path must continue to write to disk + even though new_session itself no longer saves.""" + s = new_session() + s.title = "btw: question" + s.messages = [{"role": "user", "content": "hi"}] + s.save() # mirrors api/routes.py:_handle_btw / _handle_background + assert s.path.exists() + on_disk = json.loads(s.path.read_text(encoding="utf-8")) + assert on_disk["title"] == "btw: question" + + +# ── 4. crash-safety semantics: no orphan files accumulate on the new path ── + + +def test_repeated_new_session_creates_no_disk_files(_isolate, tmp_path): + """Five news in a row produce zero disk files. Pre-fix this would have + written five orphan JSON files to SESSION_DIR.""" + session_dir = tmp_path / "sessions" + for _ in range(5): + new_session() + on_disk_jsons = [p for p in session_dir.glob("*.json") if not p.name.startswith("_")] + assert on_disk_jsons == [], ( + f"new_session() produced disk files: {[p.name for p in on_disk_jsons]}" + ) diff --git a/tests/test_provider_mismatch.py b/tests/test_provider_mismatch.py index cb8bee6ed87..85c666ca5eb 100644 --- a/tests/test_provider_mismatch.py +++ b/tests/test_provider_mismatch.py @@ -541,6 +541,16 @@ def test_api_session_is_side_effect_free_for_stale_models(): sid = created["session"]["session_id"] session_path = TEST_STATE_DIR / "sessions" / f"{sid}.json" + # POST /api/session/new no longer eagerly writes empty sessions to disk + # (#1171 follow-up). Materialise the file from the API response so the + # rest of this test, which checks that GET is side-effect-free against + # an on-disk session with a stale model, has a file to work with. + if not session_path.exists(): + session_path.parent.mkdir(parents=True, exist_ok=True) + session_path.write_text( + json.dumps(created["session"], ensure_ascii=False, indent=2), + encoding="utf-8", + ) session_data = json.loads(session_path.read_text(encoding="utf-8")) stale_model = "google/gemini-3.1-pro-preview" session_data["model"] = stale_model From c71fb4b8f554893bd4bb92f39d202b304fd1f058 Mon Sep 17 00:00:00 2001 From: Hermes Agent Date: Mon, 27 Apr 2026 23:35:53 +0000 Subject: [PATCH 2/2] chore: add v0.50.230 CHANGELOG entry --- CHANGELOG.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 936be3d48d9..79ae1dba9b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -357,6 +357,18 @@ workspace subtree) and never enumerate blocked system roots. (`api/routes.py`, `api/workspace.py`, `static/panels.js`, `static/style.css`) (partial for #616) +## [v0.50.230] — 2026-04-27 + +### Fixed +- **No disk write for empty sessions** — `new_session()` no longer eagerly writes an empty + JSON file to disk. The session lives in the in-memory `SESSIONS` dict only; the first disk + write happens at the natural "this is now a real session" moment (first user message via + `/api/chat/start`, or explicit `s.save()` in the btw/background-agent paths). Eliminates + orphan `sessions/*.json` files that accumulated on every page reload, New Conversation click, + or onboarding pass without sending a message. Crash-safety: if the process exits between + create and first message, the session is lost — since it had no messages, there is nothing + to lose. (`api/models.py`) (#1171 follow-up, #1184) + ## [v0.50.229] — 2026-04-27 ### Performance