fix(files): preserve SQLite locks during previews
Co-Authored-By: Hermes Agent / OpenAI Codex / gpt-6-sol <noreply@agents.invalid> (cherry picked from commit 60d879c125177cd4cacdbc840f16c2e048b4199c)
This commit is contained in:
@@ -296,6 +296,21 @@ def _expand_path_reference(ref: ContextReference, cwd: Path, *, allowed_root: Pa
|
||||
if is_folder:
|
||||
listing = _build_folder_listing(path, cwd, display_base=allowed_root)
|
||||
return None, f"📁 {ref.raw} ({estimate_tokens_rough(listing)} tokens)\n{listing}"
|
||||
from hermes_cli.sqlite_safe_read import LiveConnectionError, offline_file_access
|
||||
try:
|
||||
# Keep admission through every sniff, line count and text read: a connection
|
||||
# can start between a check and a later open otherwise.
|
||||
with offline_file_access(path, what="preview context reference"):
|
||||
return _expand_file_reference(ref, path, max_inline_tokens)
|
||||
except LiveConnectionError:
|
||||
return None, _on_disk_reference_block(
|
||||
ref, path, descriptor="live SQLite database file",
|
||||
reason="not previewed: raw access would cancel SQLite's POSIX locks.",
|
||||
guidance="Do not open this file directly while its database connection is live.",
|
||||
)
|
||||
|
||||
|
||||
def _expand_file_reference(ref: ContextReference, path: Path, max_inline_tokens: int | None) -> Expansion:
|
||||
if _is_binary_file(path):
|
||||
# A bare "not supported" warning was a dead end (the model gave up); the file IS
|
||||
# on disk where the agent's tools run, so hand it an actionable block instead.
|
||||
@@ -659,13 +674,17 @@ def _file_metadata(path: Path) -> str:
|
||||
return "unknown size"
|
||||
# A listing line is a summary, not content: past the cap, byte size conveys the
|
||||
# same "how big is this" without a full scan per entry.
|
||||
if _is_binary_file(path) or size > _LINE_COUNT_MAX_BYTES:
|
||||
return f"{size} bytes"
|
||||
from hermes_cli.sqlite_safe_read import LiveConnectionError, offline_file_access
|
||||
try:
|
||||
with path.open("rb") as fh:
|
||||
# UTF-8 never embeds 0x0A inside a multibyte sequence, so counting bytes
|
||||
# matches a decoded newline count while streaming instead of read_text.
|
||||
lines = sum(chunk.count(b"\n") for chunk in iter(lambda: fh.read(1 << 20), b""))
|
||||
return f"{lines + 1} lines"
|
||||
except Exception:
|
||||
# A directory preview inspects each entry separately; the registry lock
|
||||
# must cover both its binary sniff and optional line-count read.
|
||||
with offline_file_access(path, what="inspect folder entry"):
|
||||
if _is_binary_file(path) or size > _LINE_COUNT_MAX_BYTES:
|
||||
return f"{size} bytes"
|
||||
with path.open("rb") as fh:
|
||||
# UTF-8 never embeds 0x0A inside a multibyte sequence, so counting bytes
|
||||
# matches a decoded newline while streaming instead of read_text.
|
||||
lines = sum(chunk.count(b"\n") for chunk in iter(lambda: fh.read(1 << 20), b""))
|
||||
return f"{lines + 1} lines"
|
||||
except (LiveConnectionError, OSError):
|
||||
return f"{size} bytes"
|
||||
|
||||
@@ -230,7 +230,15 @@ def offline_file_access(path: Path | str, *, what: str = "read"):
|
||||
:func:`has_live_connection` and *then* doing raw I/O is a check/use race (a connection opened
|
||||
in between loses its POSIX locks to the raw ``close()``). Held only for the raw I/O."""
|
||||
with _live_lock:
|
||||
if _key(path) in _live_connections:
|
||||
key = _key(path)
|
||||
# SQLite locks the main file and its WAL shared-memory sidecar. A raw
|
||||
# close of either inode cancels this process's POSIX locks, while the
|
||||
# connection registry is keyed by the main database path.
|
||||
live = key in _live_connections or any(
|
||||
key.endswith(suffix) and key[:-len(suffix)] in _live_connections
|
||||
for suffix in ("-wal", "-shm")
|
||||
)
|
||||
if live:
|
||||
raise LiveConnectionError(
|
||||
f"Refusing to {what} {path}: a connection to it is still open "
|
||||
"in this process, and raw file access would cancel that "
|
||||
|
||||
@@ -176,11 +176,16 @@ def _fs_regular_file(path: Path) -> tuple[Path, os.stat_result]:
|
||||
|
||||
def _fs_read_bytes(target: Path, limit: Optional[int] = None) -> bytes:
|
||||
"""Read (a prefix of) ``target``; 403/400 on failure."""
|
||||
from hermes_cli.sqlite_safe_read import LiveConnectionError, offline_file_access
|
||||
try:
|
||||
if limit is None:
|
||||
return target.read_bytes()
|
||||
with target.open("rb") as handle:
|
||||
return handle.read(limit)
|
||||
# Keep admission through close; a raw close cancels this process's SQLite locks.
|
||||
with offline_file_access(target, what="preview file"):
|
||||
if limit is None:
|
||||
return target.read_bytes()
|
||||
with target.open("rb") as handle:
|
||||
return handle.read(limit)
|
||||
except LiveConnectionError as exc:
|
||||
raise HTTPException(status_code=409, detail=str(exc)) from exc
|
||||
except PermissionError:
|
||||
raise HTTPException(status_code=403, detail="File is not readable")
|
||||
except OSError as exc:
|
||||
|
||||
91
tests/agent/test_live_db_preview_locks.py
Normal file
91
tests/agent/test_live_db_preview_locks.py
Normal file
@@ -0,0 +1,91 @@
|
||||
"""File previews may not cancel SQLite's live POSIX locks (including WAL sidecars)."""
|
||||
import asyncio
|
||||
import os
|
||||
from pathlib import Path
|
||||
import sqlite3
|
||||
import subprocess
|
||||
import sys
|
||||
|
||||
import pytest
|
||||
from fastapi import HTTPException
|
||||
|
||||
from agent.context_references import _expand_path_reference, parse_context_references
|
||||
from hermes_state import SessionDB
|
||||
from hermes_cli.web_routers.files import fs_read_text
|
||||
|
||||
|
||||
@pytest.mark.linux_only
|
||||
@pytest.mark.parametrize("route,target_kind", [
|
||||
("file", "main"), ("file", "shm"), ("file", "shm_alias"), ("file", "wal"),
|
||||
("folder", "directory"), ("desktop", "main"),
|
||||
("desktop", "shm"), ("desktop", "shm_alias"), ("desktop", "wal"),
|
||||
])
|
||||
def test_preview_preserves_live_database_locks(tmp_path, route, target_kind):
|
||||
path = tmp_path / "state.db"
|
||||
text = tmp_path / "normal.txt"
|
||||
text.write_text("ordinary readable text", encoding="utf-8")
|
||||
db = SessionDB(path)
|
||||
try:
|
||||
db.create_session("preview-test", "cli")
|
||||
db.append_message("preview-test", "user", "before preview")
|
||||
shm = Path(str(path) + "-shm")
|
||||
wal = Path(str(path) + "-wal")
|
||||
alias = tmp_path / "linked-shm"
|
||||
alias.symlink_to(shm)
|
||||
target = {"main": path, "shm": shm, "shm_alias": alias,
|
||||
"wal": wal, "directory": tmp_path}[target_kind]
|
||||
conn = db._conn
|
||||
assert isinstance(conn, sqlite3.Connection)
|
||||
conn.execute("BEGIN IMMEDIATE")
|
||||
|
||||
def posix_locks(file):
|
||||
inode = file.stat().st_ino
|
||||
return sorted(line.split(": ", 1)[1] for line in Path("/proc/locks").read_text(encoding="utf-8").splitlines()
|
||||
if f":{inode} " in line and f"POSIX ADVISORY" in line
|
||||
and f" {os.getpid()} " in line)
|
||||
|
||||
def rival_locked():
|
||||
code = ("import sqlite3,sys; c=sqlite3.connect(sys.argv[1], timeout=0); "
|
||||
"c.execute('BEGIN IMMEDIATE'); c.rollback(); c.close()")
|
||||
result = subprocess.run([sys.executable, "-c", code, str(path)],
|
||||
capture_output=True, text=True, timeout=10)
|
||||
return result.returncode != 0 and "database is locked" in result.stderr
|
||||
|
||||
before = (posix_locks(path), posix_locks(shm))
|
||||
assert all(before), "fixture must hold POSIX main and WAL-sidecar locks"
|
||||
assert rival_locked(), "second process must be excluded before the preview"
|
||||
|
||||
if route == "desktop":
|
||||
with pytest.raises(HTTPException) as refused:
|
||||
asyncio.run(fs_read_text(str(target)))
|
||||
assert refused.value.status_code == 409
|
||||
assert asyncio.run(fs_read_text(str(text)))["text"] == "ordinary readable text"
|
||||
else:
|
||||
ref = parse_context_references(f"@{route}:{target}")[0]
|
||||
warning, block = _expand_path_reference(ref, tmp_path.parent)
|
||||
assert warning is None
|
||||
assert block is not None
|
||||
assert "not previewed" in block if route == "file" else "state.db" in block
|
||||
ordinary = parse_context_references(f"@file:{text}")[0]
|
||||
warning, block = _expand_path_reference(ordinary, tmp_path.parent)
|
||||
assert warning is None and block is not None and "ordinary readable text" in block
|
||||
|
||||
assert (posix_locks(path), posix_locks(shm)) == before
|
||||
assert rival_locked(), "second process entered a still-open write transaction"
|
||||
conn.rollback()
|
||||
db.append_message("preview-test", "assistant", "after preview")
|
||||
assert len(db.get_messages("preview-test")) == 2
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
|
||||
def test_closed_database_can_still_be_previewed(tmp_path):
|
||||
path = tmp_path / "offline.db"
|
||||
db = SessionDB(path)
|
||||
db.create_session("offline", "cli")
|
||||
db.close()
|
||||
ref = parse_context_references(f"@file:{path}")[0]
|
||||
warning, block = _expand_path_reference(ref, tmp_path.parent)
|
||||
assert warning is None and block is not None and "binary file" in block
|
||||
preview = asyncio.run(fs_read_text(str(path)))
|
||||
assert preview["binary"] is True and preview["byteSize"] == path.stat().st_size
|
||||
Reference in New Issue
Block a user