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
This commit is contained in:
@@ -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()
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user