From a30ccdbeab62a0132e2358b918cd09e1740b0831 Mon Sep 17 00:00:00 2001 From: yoniebans Date: Mon, 14 Sep 2026 13:51:55 +0200 Subject: [PATCH] fix(tui_gateway): keep opportunistic writes off read-only foreign handles Two write paths were still reachable through the now read-only foreign-profile handle (reported by @ehz0ah on #110718): the repo-root backfill in _discover_repos_payload raised and was swallowed per RPC, silently dropping the persistence; the Bot Chat unarchive in session.list's exact-title lookup failed the RPC with error 5006. Backfill now skips on read-only handles (that profile's own gateway backfills on its refreshes); the unarchive escalates to a short-lived registry writer for the rare recoverable-archive case. Reported-by: ehz0ah --- tests/tui_gateway/test_profile_db_readonly.py | 47 ++++++++++++++++++- tui_gateway/methods_projects.py | 5 +- tui_gateway/methods_session.py | 20 +++++++- 3 files changed, 69 insertions(+), 3 deletions(-) diff --git a/tests/tui_gateway/test_profile_db_readonly.py b/tests/tui_gateway/test_profile_db_readonly.py index da9b551269..016c005639 100644 --- a/tests/tui_gateway/test_profile_db_readonly.py +++ b/tests/tui_gateway/test_profile_db_readonly.py @@ -5,7 +5,9 @@ mode, profile switcher) used to acquire() a WRITER on that profile's state.db pe close it in the handler's finally. Reads never need that lock, and the writer's close participated in the deleted-WAL incident class. Read paths now open read-only, mirroring hermes_cli.web_routers.profiles._read_profile_db; the few RPCs that genuinely write -(move-cwd, delete, set_hidden, foreign import) opt in with writer=True. +(move-cwd, delete, set_hidden, foreign import) opt in with writer=True, and the two +opportunistic writes reachable from read RPCs (repo-root backfill, Bot Chat unarchive) +either skip or escalate to a short-lived writer instead of writing on the reader. """ from __future__ import annotations @@ -43,3 +45,46 @@ def test_foreign_profile_db_writer_opt_in(monkeypatch, tmp_path): assert db is not None assert db.read_only is False assert db.set_session_title("seed", "renamed") is True + + +def test_discover_repos_payload_skips_backfill_on_read_only(monkeypatch, tmp_path): + """The repo-root backfill UPDATE must not be attempted (and swallowed) on a reader.""" + _bind_foreign(monkeypatch, tmp_path) + with server._profile_db({"profile": "code"}) as db: + calls = [] + monkeypatch.setattr(type(db), "backfill_repo_roots", lambda self, m: calls.append(m), raising=False) + server._discover_repos_payload(db, backfill=True, include_cached=False) + assert calls == [] + # Same call on a writable handle still backfills. + with server._profile_db({"profile": "code"}, writer=True) as db: + calls = [] + monkeypatch.setattr(type(db), "backfill_repo_roots", lambda self, m: calls.append(m), raising=False) + server._discover_repos_payload(db, backfill=True, include_cached=False) + assert len(calls) == 1 + + +def test_bot_chat_unarchive_escalates_to_writer(monkeypatch, tmp_path): + """An archived-by-accident Bot Chat found via an exact-title lookup on a READ-ONLY foreign + handle is still resurrected: the write goes through a short-lived registry writer.""" + from tools.bot_mode_probe import BOT_CHAT_TITLE + + foreign = tmp_path / "profiles" / "code" + foreign.mkdir(parents=True, exist_ok=True) + db = SessionDB(db_path=foreign / "state.db") + db.create_session(session_id="bot", source="cli", model="m") + db.set_session_title("bot", BOT_CHAT_TITLE) + db.end_session("bot", "ws_orphan_reap") + db.set_session_archived("bot", True) + db.close() + monkeypatch.setattr(server, "_profile_home", lambda name: foreign if (name or "").strip() == "code" else None) + + with server._profile_db({"profile": "code"}) as ro_db: + assert ro_db.read_only is True + resp = server._session_list_by_title("rid", ro_db, BOT_CHAT_TITLE) + assert resp["result"]["sessions"], "archived Bot Chat was not resurrected through the writer escalation" + + check = SessionDB(db_path=foreign / "state.db", read_only=True) + try: + assert not check.get_session("bot").get("archived") + finally: + check.close() diff --git a/tui_gateway/methods_projects.py b/tui_gateway/methods_projects.py index f8ba640e22..09bc1c2d1c 100644 --- a/tui_gateway/methods_projects.py +++ b/tui_gateway/methods_projects.py @@ -286,7 +286,10 @@ def _discover_repos_payload( agg = _agg(root) agg["sessions"] += int(row.get("sessions") or 0) agg["last_active"] = max(agg["last_active"], float(row.get("last_active") or 0)) - if backfill: + # A read-only handle (foreign-profile RPC) must not attempt the persistence write: it would + # raise and be swallowed here, silently dropping the backfill. That profile's own gateway + # backfills on its own refreshes. + if backfill and not getattr(db, "read_only", False): try: db.backfill_repo_roots(cwd_to_root) except Exception: diff --git a/tui_gateway/methods_session.py b/tui_gateway/methods_session.py index b14d20594b..b307f0e814 100644 --- a/tui_gateway/methods_session.py +++ b/tui_gateway/methods_session.py @@ -389,6 +389,24 @@ def _(rid, params: dict) -> dict: "profile_name": _response_profile_name(profile)}}) +def _unarchive_recoverable(db, session_id: str) -> bool: + """``unarchive_recoverable_session`` that works on a read-only listing handle (foreign profile): + the rare write escalates to a short-lived registry writer instead of writing on the reader.""" + if not getattr(db, "read_only", False): + return db.unarchive_recoverable_session(session_id) + from hermes_state_registry import acquire + try: + wdb = acquire(db.db_path) + except Exception: + logger.warning("Bot Chat unarchive skipped: writer unavailable for %s", db.db_path, exc_info=True) + return False + try: + return wdb.unarchive_recoverable_session(session_id) + finally: + with contextlib.suppress(Exception): + wdb.close() + + def _session_list_by_title(rid, db, title_lookup: str) -> dict: """EXACT-title lookup (title as identity), window-free on purpose (a busy profile's windowed listing can push the row out). Hidden rows resolve (canonical chats are born hidden); archived / deny-listed do not; @@ -398,7 +416,7 @@ def _session_list_by_title(rid, db, title_lookup: str) -> dict: from tools.bot_mode_probe import BOT_CHAT_TITLE # A Bot Chat archived by the ws-orphan reaper / agent_close is an accident (the desktop would mint # replacements forever): resurrect recoverable reasons only. Re-fetch by ID — title is not UNIQUE. - if title_lookup == BOT_CHAT_TITLE and db.unarchive_recoverable_session(row["id"]): + if title_lookup == BOT_CHAT_TITLE and _unarchive_recoverable(db, row["id"]): # The canonical Bot Chat is identity-scoped: an archive stamped by the ws-orphan reaper or older # agent cleanup (ws_orphan_reap / agent_close) is an accident, not user intent, and hiding the # row here makes the desktop mint transient replacements forever (#92687). Resurrect it — same