fix: file manager falls back to state.db for external Telegram/CLI sessions (#3280)
This commit is contained in:
committed by
nesquena-hermes
parent
fae5ada40d
commit
92bddca3c7
@@ -2655,6 +2655,68 @@ def _refresh_index_rows_from_sidecar_metadata(sessions: list[dict]) -> list[dict
|
||||
return out
|
||||
|
||||
|
||||
def state_db_has_session(sid: str) -> bool:
|
||||
"""Return True when ``sid`` exists in the active state.db sessions table.
|
||||
|
||||
Used by file-manager handlers to fall back to a state.db lookup when
|
||||
``get_session`` raises ``KeyError`` because the session was created by
|
||||
Telegram/CLI (external) rather than the WebUI (issue #3280). The state.db
|
||||
schema stores only metadata (id/title/model/source/...), not a workspace
|
||||
path — the workspace is shared across session storage backends and is
|
||||
resolved separately via ``get_last_workspace()``.
|
||||
"""
|
||||
if not sid:
|
||||
return False
|
||||
try:
|
||||
import sqlite3
|
||||
except ImportError:
|
||||
return False
|
||||
db_path = _active_state_db_path()
|
||||
if not db_path.exists():
|
||||
return False
|
||||
try:
|
||||
with closing(sqlite3.connect(str(db_path))) as conn:
|
||||
cur = conn.cursor()
|
||||
cur.execute("SELECT 1 FROM sessions WHERE id = ? LIMIT 1", (str(sid),))
|
||||
return cur.fetchone() is not None
|
||||
except Exception:
|
||||
return False
|
||||
|
||||
|
||||
class _ExternalSessionView:
|
||||
"""Minimal session-shaped view for external (Telegram/CLI) sessions.
|
||||
|
||||
Only exposes the fields file-manager handlers need (``session_id`` and
|
||||
``workspace``). The workspace falls back to the WebUI's last-used
|
||||
workspace because state.db does not persist a per-session workspace path
|
||||
and the file browser is intentionally workspace-scoped, not
|
||||
session-storage-scoped (issue #3280).
|
||||
"""
|
||||
|
||||
__slots__ = ("session_id", "workspace")
|
||||
|
||||
def __init__(self, session_id: str, workspace: str):
|
||||
self.session_id = session_id
|
||||
self.workspace = workspace
|
||||
|
||||
|
||||
def get_session_for_file_ops(sid: str):
|
||||
"""Return a session-like object for file-manager handlers.
|
||||
|
||||
Tries ``get_session`` first (preserves all existing behavior for WebUI
|
||||
sessions). If that raises ``KeyError``, checks state.db; when the session
|
||||
exists there, returns an ``_ExternalSessionView`` whose ``workspace`` is
|
||||
the active WebUI workspace. If neither has the session, re-raises
|
||||
``KeyError`` so callers continue to return their existing 404.
|
||||
"""
|
||||
try:
|
||||
return get_session(sid, metadata_only=True)
|
||||
except KeyError:
|
||||
if state_db_has_session(sid):
|
||||
return _ExternalSessionView(str(sid), str(get_last_workspace()))
|
||||
raise
|
||||
|
||||
|
||||
def _active_state_db_path() -> Path:
|
||||
"""Return state.db for the active Hermes profile, degrading to HERMES_HOME."""
|
||||
try:
|
||||
|
||||
@@ -2801,6 +2801,7 @@ def _keep_latest_messaging_session_per_source(
|
||||
from api.models import (
|
||||
Session,
|
||||
get_session,
|
||||
get_session_for_file_ops,
|
||||
new_session,
|
||||
all_sessions,
|
||||
title_from,
|
||||
@@ -8579,7 +8580,7 @@ def _handle_folder_download(handler, parsed):
|
||||
if not sid:
|
||||
return bad(handler, "session_id is required")
|
||||
try:
|
||||
s = get_session(sid)
|
||||
s = get_session_for_file_ops(sid)
|
||||
except KeyError:
|
||||
return bad(handler, "Session not found", 404)
|
||||
|
||||
@@ -8652,7 +8653,7 @@ def _handle_file_raw(handler, parsed):
|
||||
if not sid:
|
||||
return bad(handler, "session_id is required")
|
||||
try:
|
||||
s = get_session(sid)
|
||||
s = get_session_for_file_ops(sid)
|
||||
except KeyError:
|
||||
return bad(handler, "Session not found", 404)
|
||||
rel = qs.get("path", [""])[0]
|
||||
@@ -8691,7 +8692,7 @@ def _handle_file_read(handler, parsed):
|
||||
if not sid:
|
||||
return bad(handler, "session_id is required")
|
||||
try:
|
||||
s = get_session(sid)
|
||||
s = get_session_for_file_ops(sid)
|
||||
except KeyError:
|
||||
return bad(handler, "Session not found", 404)
|
||||
rel = qs.get("path", [""])[0]
|
||||
@@ -11012,7 +11013,7 @@ def _handle_file_delete(handler, body):
|
||||
except ValueError as e:
|
||||
return bad(handler, str(e))
|
||||
try:
|
||||
s = get_session(body["session_id"])
|
||||
s = get_session_for_file_ops(body["session_id"])
|
||||
except KeyError:
|
||||
return bad(handler, "Session not found", 404)
|
||||
try:
|
||||
@@ -11036,7 +11037,7 @@ def _handle_file_save(handler, body):
|
||||
except ValueError as e:
|
||||
return bad(handler, str(e))
|
||||
try:
|
||||
s = get_session(body["session_id"])
|
||||
s = get_session_for_file_ops(body["session_id"])
|
||||
except KeyError:
|
||||
return bad(handler, "Session not found", 404)
|
||||
try:
|
||||
@@ -11059,7 +11060,7 @@ def _handle_file_create(handler, body):
|
||||
except ValueError as e:
|
||||
return bad(handler, str(e))
|
||||
try:
|
||||
s = get_session(body["session_id"])
|
||||
s = get_session_for_file_ops(body["session_id"])
|
||||
except KeyError:
|
||||
return bad(handler, "Session not found", 404)
|
||||
try:
|
||||
@@ -11081,7 +11082,7 @@ def _handle_file_rename(handler, body):
|
||||
except ValueError as e:
|
||||
return bad(handler, str(e))
|
||||
try:
|
||||
s = get_session(body["session_id"])
|
||||
s = get_session_for_file_ops(body["session_id"])
|
||||
except KeyError:
|
||||
return bad(handler, "Session not found", 404)
|
||||
try:
|
||||
@@ -11107,7 +11108,7 @@ def _handle_create_dir(handler, body):
|
||||
except ValueError as e:
|
||||
return bad(handler, str(e))
|
||||
try:
|
||||
s = get_session(body["session_id"])
|
||||
s = get_session_for_file_ops(body["session_id"])
|
||||
except KeyError:
|
||||
return bad(handler, "Session not found", 404)
|
||||
try:
|
||||
@@ -11128,7 +11129,7 @@ def _handle_file_reveal(handler, body):
|
||||
except ValueError as e:
|
||||
return bad(handler, str(e))
|
||||
try:
|
||||
s = get_session(body["session_id"])
|
||||
s = get_session_for_file_ops(body["session_id"])
|
||||
except KeyError:
|
||||
return bad(handler, "Session not found", 404)
|
||||
try:
|
||||
@@ -11175,7 +11176,7 @@ def _handle_file_path(handler, body):
|
||||
except ValueError as e:
|
||||
return bad(handler, str(e))
|
||||
try:
|
||||
s = get_session(body["session_id"])
|
||||
s = get_session_for_file_ops(body["session_id"])
|
||||
except KeyError:
|
||||
return bad(handler, "Session not found", 404)
|
||||
try:
|
||||
@@ -11205,7 +11206,7 @@ def _handle_file_open_vscode(handler, body):
|
||||
except ValueError as e:
|
||||
return bad(handler, str(e))
|
||||
try:
|
||||
s = get_session(body["session_id"])
|
||||
s = get_session_for_file_ops(body["session_id"])
|
||||
except KeyError:
|
||||
return bad(handler, "Session not found", 404)
|
||||
try:
|
||||
|
||||
185
tests/test_file_manager_external_session.py
Normal file
185
tests/test_file_manager_external_session.py
Normal file
@@ -0,0 +1,185 @@
|
||||
"""Regression tests for #3280 — file manager falls back to state.db for
|
||||
external (Telegram/CLI) sessions instead of returning 404.
|
||||
|
||||
Covers:
|
||||
(a) WebUI session — existing behavior preserved (get_session path).
|
||||
(b) state.db-only session — fallback returns a workspace-bearing view.
|
||||
(c) Unknown session — KeyError still propagates so callers 404.
|
||||
(d) Static check: every file-manager handler in api/routes.py calls
|
||||
get_session_for_file_ops, not the raw get_session.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
import sqlite3
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
ROOT = Path(__file__).resolve().parents[1]
|
||||
ROUTES_PY = ROOT / "api" / "routes.py"
|
||||
|
||||
|
||||
FILE_HANDLERS = [
|
||||
"_handle_folder_download",
|
||||
"_handle_file_raw",
|
||||
"_handle_file_read",
|
||||
"_handle_file_delete",
|
||||
"_handle_file_save",
|
||||
"_handle_file_create",
|
||||
"_handle_file_rename",
|
||||
"_handle_create_dir",
|
||||
"_handle_file_reveal",
|
||||
"_handle_file_path",
|
||||
"_handle_file_open_vscode",
|
||||
]
|
||||
|
||||
|
||||
def _handler_body(src: str, name: str) -> str:
|
||||
start = src.index(f"def {name}(")
|
||||
# next top-level def or class
|
||||
m = re.search(r"\n(?:def |class )", src[start + 1 :])
|
||||
end = (start + 1 + m.start()) if m else len(src)
|
||||
return src[start:end]
|
||||
|
||||
|
||||
def test_routes_file_handlers_use_fallback():
|
||||
src = ROUTES_PY.read_text(encoding="utf-8")
|
||||
assert "get_session_for_file_ops" in src, "fallback helper must be imported"
|
||||
missing = []
|
||||
for name in FILE_HANDLERS:
|
||||
body = _handler_body(src, name)
|
||||
# Must not call get_session(...) directly inside the handler.
|
||||
# (get_session_for_file_ops also contains "get_session(" as a substring,
|
||||
# so check word-boundary occurrences.)
|
||||
bare = re.findall(r"(?<!_)\bget_session\(", body)
|
||||
# Strip occurrences that are actually get_session_for_file_ops( — the
|
||||
# regex above already excludes underscore prefix, so any remaining
|
||||
# match is a raw get_session call.
|
||||
if bare:
|
||||
missing.append(name)
|
||||
assert not missing, f"raw get_session() still used in: {missing}"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Functional tests against api.models.get_session_for_file_ops
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
pytestmark_models = pytest.mark.requires_agent_modules
|
||||
|
||||
|
||||
def _make_state_db(path: Path, sid: str) -> None:
|
||||
conn = sqlite3.connect(str(path))
|
||||
conn.executescript(
|
||||
"""
|
||||
CREATE TABLE sessions (
|
||||
id TEXT PRIMARY KEY,
|
||||
title TEXT,
|
||||
model TEXT,
|
||||
message_count INTEGER DEFAULT 0,
|
||||
started_at TEXT,
|
||||
source TEXT
|
||||
);
|
||||
CREATE TABLE messages (
|
||||
id INTEGER PRIMARY KEY AUTOINCREMENT,
|
||||
session_id TEXT,
|
||||
role TEXT,
|
||||
content TEXT,
|
||||
timestamp TEXT
|
||||
);
|
||||
"""
|
||||
)
|
||||
conn.execute(
|
||||
"INSERT INTO sessions (id, title, model, message_count, started_at, source) "
|
||||
"VALUES (?, 'telegram session', 'gpt-x', 1, '2026-01-01T00:00:00Z', 'telegram')",
|
||||
(sid,),
|
||||
)
|
||||
conn.commit()
|
||||
conn.close()
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def models_module():
|
||||
return pytest.importorskip("api.models")
|
||||
|
||||
|
||||
def test_get_session_for_file_ops_webui_passthrough(models_module, monkeypatch):
|
||||
"""(a) WebUI session — delegates to get_session, no state.db consulted."""
|
||||
sentinel = object()
|
||||
called = {"get_session": 0, "state_db": 0}
|
||||
|
||||
def fake_get_session(sid, metadata_only=False):
|
||||
called["get_session"] += 1
|
||||
return sentinel
|
||||
|
||||
def fake_has(_sid):
|
||||
called["state_db"] += 1
|
||||
return True
|
||||
|
||||
monkeypatch.setattr(models_module, "get_session", fake_get_session)
|
||||
monkeypatch.setattr(models_module, "state_db_has_session", fake_has)
|
||||
result = models_module.get_session_for_file_ops("webui-sid")
|
||||
assert result is sentinel
|
||||
assert called == {"get_session": 1, "state_db": 0}
|
||||
|
||||
|
||||
def test_get_session_for_file_ops_state_db_fallback(
|
||||
models_module, monkeypatch, tmp_path
|
||||
):
|
||||
"""(b) state.db-only session — returns view with workspace populated."""
|
||||
db = tmp_path / "state.db"
|
||||
_make_state_db(db, "tg-123")
|
||||
workspace = tmp_path / "ws"
|
||||
workspace.mkdir()
|
||||
(workspace / "hello.txt").write_text("hi from telegram session")
|
||||
|
||||
def raise_key(sid, metadata_only=False):
|
||||
raise KeyError(sid)
|
||||
|
||||
monkeypatch.setattr(models_module, "get_session", raise_key)
|
||||
monkeypatch.setattr(models_module, "_active_state_db_path", lambda: db)
|
||||
monkeypatch.setattr(
|
||||
models_module, "get_last_workspace", lambda: str(workspace)
|
||||
)
|
||||
|
||||
view = models_module.get_session_for_file_ops("tg-123")
|
||||
assert view.session_id == "tg-123"
|
||||
assert Path(view.workspace) == workspace
|
||||
# The workspace is real and readable — file-manager handlers will
|
||||
# successfully serve files relative to it instead of returning 404.
|
||||
assert (Path(view.workspace) / "hello.txt").read_text() == "hi from telegram session"
|
||||
|
||||
|
||||
def test_get_session_for_file_ops_unknown_session_raises(
|
||||
models_module, monkeypatch, tmp_path
|
||||
):
|
||||
"""(c) Unknown session — KeyError propagates so callers still 404."""
|
||||
db = tmp_path / "state.db"
|
||||
_make_state_db(db, "tg-123")
|
||||
|
||||
def raise_key(sid, metadata_only=False):
|
||||
raise KeyError(sid)
|
||||
|
||||
monkeypatch.setattr(models_module, "get_session", raise_key)
|
||||
monkeypatch.setattr(models_module, "_active_state_db_path", lambda: db)
|
||||
monkeypatch.setattr(models_module, "get_last_workspace", lambda: str(tmp_path))
|
||||
|
||||
with pytest.raises(KeyError):
|
||||
models_module.get_session_for_file_ops("does-not-exist")
|
||||
|
||||
|
||||
def test_state_db_has_session_missing_db(models_module, monkeypatch, tmp_path):
|
||||
monkeypatch.setattr(
|
||||
models_module, "_active_state_db_path", lambda: tmp_path / "missing.db"
|
||||
)
|
||||
assert models_module.state_db_has_session("any") is False
|
||||
|
||||
|
||||
def test_state_db_has_session_present(models_module, monkeypatch, tmp_path):
|
||||
db = tmp_path / "state.db"
|
||||
_make_state_db(db, "cli-9")
|
||||
monkeypatch.setattr(models_module, "_active_state_db_path", lambda: db)
|
||||
assert models_module.state_db_has_session("cli-9") is True
|
||||
assert models_module.state_db_has_session("nope") is False
|
||||
Reference in New Issue
Block a user