fix(state): VACUUM admission counts live holders only; argv proves the home, not the install
Independent review found the previous commit could make the unbounded-growth symptom it fixes PERMANENT, and that its argv narrowing re-opened #92401 inside a single install. - other_generations_for_path() counts only LIVE generations. A retired one is already write-fenced (StateDbReplacedError, close-time checkpoint disabled) and leaves the registry only when its holder releases -- which a gateway handle does not do before shutdown. One inode replacement (repair swap, backup restore, snapshot) therefore skipped auto-VACUUM for that path for the whole process lifetime. - _argv_scoped_to_other_home is ranked evidence now. argv[0] is the SHARED install binary for every profile on a host, so it is neutral, never proof of a hold; the process's own --hermes-home / HERMES_HOME= / --profile / -p selection decides which home it serves, and a token naming ANOTHER profile's store is tested before any own-prefix check. argv[0] also stops dismissing a holder of a store whose home is not part of an install layout (a custom HERMES_HOME is served BY the binary under ~/.hermes). - install_root comes from hermes_constants.named_profile_home, not basename(parent) == "profiles": an arbitrary <X>/profiles/<n>/ tree no longer promotes all of <X> to "ours". - The launch profile keeps its gateway.sessions_dir override when its own store is pruned; every other served profile prunes under its own <home>/sessions. Pruning under the wrong dir orphaned transcripts forever. - glob.escape on the request_dump_<id>_* sweep (pre-existing). Tests: the retired-generation test asserted the starvation mechanism; it is replaced by the invariant (a live sibling defers VACUUM) plus a red-on-base test that a retired, write-fenced generation does not. New argv cases cover the shared binary with -p other, another profile's store token, and a non-Hermes <X>/profiles/ tree; the housekeeping fixture now asserts the unpinned store still resolves inside the sandbox before yielding.
This commit is contained in:
@@ -3710,10 +3710,11 @@ class GatewayRunner(
|
||||
# state.db was never pruned or vacuumed by anybody, and the launch profile's
|
||||
# retention_days/auto_prune decided whether it happened at all.
|
||||
from gateway.run_profile_reconcile import _for_each_served_profile
|
||||
_launch_sessions = _launch_sessions_dir(self.config) # resolved OUTSIDE any profile scope
|
||||
_housekeeping_chore(
|
||||
"state.db startup maintenance",
|
||||
lambda: _for_each_served_profile(
|
||||
self, lambda _label: _housekeeping_state_db_maintenance()))
|
||||
self, lambda _label: _housekeeping_state_db_maintenance(_launch_sessions)))
|
||||
# Checkpoint store pruning is a housekeeping chore (``_housekeeping_checkpoint_prune``), not a
|
||||
# constructor step: its ``git gc`` repacks the whole store (tens of seconds on a GB store) and
|
||||
# here it ran before the control socket, adapters and the code_sha stamp — so the first
|
||||
@@ -4611,7 +4612,33 @@ def _housekeeping_org_skill_sync() -> None:
|
||||
maybe_pull_org_skills()
|
||||
|
||||
|
||||
def _housekeeping_state_db_maintenance() -> None:
|
||||
def _launch_sessions_dir(config) -> Optional[Tuple[Path, Path]]:
|
||||
"""``(launch home, its configured transcript dir)``, or ``None`` when the gateway carries none.
|
||||
|
||||
MUST be called outside any profile scope — ``get_hermes_home()`` is what identifies the launch
|
||||
home. Consumed by :func:`_profile_sessions_dir`.
|
||||
"""
|
||||
sessions_dir = getattr(config, "sessions_dir", None)
|
||||
if sessions_dir is None:
|
||||
return None
|
||||
return get_hermes_home(), Path(sessions_dir)
|
||||
|
||||
|
||||
def _profile_sessions_dir(launch: Optional[Tuple[Path, Path]]) -> Path:
|
||||
"""Transcript dir of the profile currently in scope.
|
||||
|
||||
``gateway.sessions_dir`` overrides the LAUNCH profile's transcript dir only; every other served
|
||||
profile keeps ``<home>/sessions``. Hardcoding ``<home>/sessions`` for the launch home too wrote
|
||||
transcripts to the configured dir while the prune unlinked under the default one, orphaning
|
||||
every pruned session's ``.json``/``.jsonl``/``request_dump_*`` forever.
|
||||
"""
|
||||
home = get_hermes_home()
|
||||
if launch is not None and Path(launch[0]) == home:
|
||||
return Path(launch[1])
|
||||
return home / "sessions"
|
||||
|
||||
|
||||
def _housekeeping_state_db_maintenance(launch: Optional[Tuple[Path, Path]] = None) -> None:
|
||||
"""Stale-session auto-archive plus auto-prune/VACUUM for ONE profile's state.db; both are gated
|
||||
by sessions.min_interval_hours (VACUUM additionally by its own throttles). Opens its own
|
||||
SessionDB — SQLite connections are thread-bound.
|
||||
@@ -4620,7 +4647,8 @@ def _housekeeping_state_db_maintenance() -> None:
|
||||
resolve through the active scope, so an unscoped run swept only the LAUNCH profile's store with
|
||||
the LAUNCH profile's retention settings and a multiplexed secondary was never archived, pruned
|
||||
or vacuumed by anyone — the dashboard/serve trigger defers to the gateway for every profile a
|
||||
gateway owns (``web_server_sessions``)."""
|
||||
gateway owns (``web_server_sessions``). *launch* carries the launch home's configured transcript
|
||||
dir (:func:`_launch_sessions_dir`) so its override still governs its own profile."""
|
||||
from hermes_cli.config import load_config as _load_full_config
|
||||
from hermes_state_registry import acquire, release_or_close
|
||||
_sess_cfg = (_load_full_config().get("sessions") or {})
|
||||
@@ -4638,8 +4666,7 @@ def _housekeeping_state_db_maintenance() -> None:
|
||||
min_interval_hours=int(_sess_cfg.get("min_interval_hours", 24)),
|
||||
min_vacuum_interval_days=int(_sess_cfg.get("min_vacuum_interval_days", 30)),
|
||||
vacuum=bool(_sess_cfg.get("vacuum_after_prune", True)),
|
||||
# This profile's own transcript dir, not the launch profile's ``config.sessions_dir``.
|
||||
sessions_dir=get_hermes_home() / "sessions")
|
||||
sessions_dir=_profile_sessions_dir(launch))
|
||||
finally:
|
||||
release_or_close(_adb)
|
||||
|
||||
@@ -4734,7 +4761,11 @@ def _start_gateway_housekeeping(
|
||||
(60, "Curator tick", profile_scoped_chore(runner, _housekeeping_curator)),
|
||||
(60, "Sync pull tick", profile_scoped_chore(runner, _housekeeping_skill_sync)),
|
||||
(60, "Org sync pull tick", profile_scoped_chore(runner, _housekeeping_org_skill_sync)),
|
||||
(60, "state.db maintenance tick", profile_scoped_chore(runner, _housekeeping_state_db_maintenance)),
|
||||
(60, "state.db maintenance tick", profile_scoped_chore(
|
||||
runner,
|
||||
# Default-bound now, i.e. OUTSIDE any profile scope: this is the launch home's override.
|
||||
lambda _launch=_launch_sessions_dir(getattr(runner, "config", None)):
|
||||
_housekeeping_state_db_maintenance(_launch))),
|
||||
(1, "Deferred FTS retry tick", _housekeeping_deferred_fts_retry),
|
||||
(1, "gateway housekeeping memory trim", _housekeeping_memory_trim),
|
||||
(1, "MCP config reconcile", _mcp_config_reconciler(runner)),
|
||||
|
||||
@@ -143,6 +143,116 @@ def canonical_sqlite_path(path: str) -> str:
|
||||
return os.path.normcase(os.path.abspath(path.removesuffix(" (deleted)")))
|
||||
|
||||
|
||||
_HOME_FLAGS = ("--hermes-home",)
|
||||
_PROFILE_FLAGS = ("--profile", "-p")
|
||||
_STATE_DB_NAMES = ("state.db", "state.db-wal", "state.db-shm")
|
||||
|
||||
|
||||
def _norm_path(value: str) -> str:
|
||||
return os.path.normcase(os.path.normpath(value))
|
||||
|
||||
|
||||
def _argv_flag_value(argv: Sequence[str], flags: Sequence[str]) -> Optional[str]:
|
||||
"""Last ``--flag X`` / ``--flag=X`` value, token-exact (``--profile timothy`` is not ``tim``)."""
|
||||
value: Optional[str] = None
|
||||
index, count = 0, len(argv)
|
||||
while index < count:
|
||||
token = argv[index]
|
||||
if isinstance(token, str):
|
||||
if token in flags and index + 1 < count and isinstance(argv[index + 1], str):
|
||||
value = argv[index + 1]
|
||||
index += 2
|
||||
continue
|
||||
for flag in flags:
|
||||
if token.startswith(flag + "="):
|
||||
value = token[len(flag) + 1:]
|
||||
break
|
||||
index += 1
|
||||
return value
|
||||
|
||||
|
||||
def _argv_env_home(argv: Sequence[str]) -> Optional[str]:
|
||||
"""``HERMES_HOME=<path>`` env-style assignment on the argv (``env HERMES_HOME=… hermes …``)."""
|
||||
for token in reversed(list(argv)):
|
||||
if isinstance(token, str) and token.startswith("HERMES_HOME="):
|
||||
return token[len("HERMES_HOME="):]
|
||||
return None
|
||||
|
||||
|
||||
def _store_install_layout(this_home: str) -> Tuple[Optional[str], Optional[str]]:
|
||||
"""``(<install root>, <our profile name>)`` for the home holding the store, else ``(None, None)``.
|
||||
|
||||
Derived with the canonical ``named_profile_home`` predicate, never a ``basename == "profiles"``
|
||||
string test: an arbitrary ``<X>/profiles/<n>/`` tree is not a Hermes install, and promoting
|
||||
``<X>`` to "ours" swallows an unrelated instance living under it — the literal two-instance
|
||||
shape of #92401. The root store (``~/.hermes/state.db``) is its own root with no profile name,
|
||||
so ANY named-profile selection contradicts it.
|
||||
"""
|
||||
try:
|
||||
from hermes_constants import named_profile_home
|
||||
|
||||
profile_home = named_profile_home(this_home)
|
||||
if profile_home is not None:
|
||||
return os.path.abspath(str(profile_home.parent.parent)), profile_home.name
|
||||
if os.path.basename(this_home) == ".hermes":
|
||||
return os.path.abspath(this_home), None
|
||||
except Exception: # constants import/resolution must never break a holder scan
|
||||
logger.debug("Could not classify the install layout of %s", this_home, exc_info=True)
|
||||
return None, None
|
||||
|
||||
|
||||
def _names_other_profile(normalized: str, install_root: Optional[str], our_profile: Optional[str]) -> bool:
|
||||
"""True when the token is under ``<install root>/profiles/<name>`` for a name that is not ours."""
|
||||
if install_root is None:
|
||||
return False
|
||||
prefix = _norm_path(os.path.join(install_root, "profiles")) + os.sep
|
||||
if not normalized.startswith(prefix):
|
||||
return False
|
||||
name = normalized[len(prefix):].split(os.sep, 1)[0]
|
||||
return bool(name) and name != (os.path.normcase(our_profile) if our_profile else None)
|
||||
|
||||
|
||||
def _argv_home_selection(
|
||||
argv: Sequence[str], this_home: str, install_root: Optional[str], our_profile: Optional[str]
|
||||
) -> Optional[str]:
|
||||
"""``"ours"``/``"other"``/``None`` from the process's OWN profile/home selection.
|
||||
|
||||
Under one process per host the shared binary path proves nothing about which home a process
|
||||
serves; its ``--hermes-home``/``HERMES_HOME=``/``--profile``/``-p`` selection does. Same
|
||||
token-exact parsers ``gateway/run.py::_argv_contradicts_home`` uses.
|
||||
"""
|
||||
home_value = _argv_flag_value(argv, _HOME_FLAGS) or _argv_env_home(argv)
|
||||
if home_value:
|
||||
return "ours" if _norm_path(home_value) == _norm_path(this_home) else "other"
|
||||
profile_value = _argv_flag_value(argv, _PROFILE_FLAGS)
|
||||
if profile_value:
|
||||
if our_profile is not None:
|
||||
return "ours" if profile_value == our_profile else "other"
|
||||
# Root/custom home: any explicit named profile selects a different home.
|
||||
return "ours" if (install_root is not None and profile_value == "default") else "other"
|
||||
return None
|
||||
|
||||
|
||||
def _argv_path_tokens(argv: Sequence[str]) -> List[Tuple[int, str]]:
|
||||
"""``(argv index, normalized absolute path)`` for every path-bearing token."""
|
||||
tokens: List[Tuple[int, str]] = []
|
||||
for index, token in enumerate(argv):
|
||||
if not isinstance(token, str):
|
||||
continue
|
||||
if token.startswith("/"):
|
||||
path_token = token
|
||||
elif token.startswith("-") and "=" in token:
|
||||
# ``--db=/abs/path``-style options carry a path value; anchor on
|
||||
# the text after '=' so normpath does not prepend the option.
|
||||
value = token.split("=", 1)[1]
|
||||
path_token = value if value.startswith("/") else None
|
||||
else:
|
||||
path_token = None
|
||||
if path_token is not None:
|
||||
tokens.append((index, _norm_path(path_token)))
|
||||
return tokens
|
||||
|
||||
|
||||
def _argv_scoped_to_other_home(argv: Sequence[str], db_path: Path) -> bool:
|
||||
"""Return whether argv proves the process belongs to a DIFFERENT instance.
|
||||
|
||||
@@ -157,61 +267,55 @@ def _argv_scoped_to_other_home(argv: Sequence[str], db_path: Path) -> bool:
|
||||
handles). Ambiguous argv without absolute-path tokens returns False and
|
||||
keeps the fail-closed suspicion.
|
||||
|
||||
One process per host serves EVERY profile, so a token naming the install
|
||||
root is not evidence of another instance when ``db_path`` is a profile
|
||||
store under it: a default-launched multiplexer holds
|
||||
``<root>/profiles/<name>/state.db`` while its argv mentions only
|
||||
``<root>``. Only a token under a DIFFERENT install root still counts.
|
||||
Evidence is ranked, because one host now runs ONE process for every profile:
|
||||
|
||||
1. A token naming our state.db or a sidecar exactly — definitive, ours.
|
||||
2. The process's own ``--hermes-home``/``HERMES_HOME=``/``--profile``/``-p``
|
||||
selection — that is what decides which home a multiplexer serves.
|
||||
3. Path tokens. A token under ``<install root>/profiles/<other>`` is another
|
||||
profile's store even though it sits under our root; a token that names only
|
||||
the SHARED install root is NEUTRAL (it is the same binary for every profile,
|
||||
so it can neither prove nor disprove a hold); ``argv[0]`` locates the INSTALL,
|
||||
not the home, so it is not other-home evidence for a store whose home is not
|
||||
part of an install layout (a custom ``HERMES_HOME`` is served BY the binary
|
||||
under ``~/.hermes`` — dismissing on it admits maintenance under a live writer).
|
||||
"""
|
||||
db_path_str = os.path.abspath(os.fspath(db_path))
|
||||
this_home = os.path.dirname(db_path_str)
|
||||
# ``<root>/profiles/<name>/state.db`` -> ``<root>``; the multiplexer scoped to the
|
||||
# install root is a candidate holder of every profile store beneath it.
|
||||
profiles_dir = os.path.dirname(this_home)
|
||||
install_root = (
|
||||
os.path.dirname(profiles_dir)
|
||||
if os.path.basename(profiles_dir) == "profiles"
|
||||
else None
|
||||
)
|
||||
ours = {
|
||||
install_root, our_profile = _store_install_layout(this_home)
|
||||
sidecars = {
|
||||
os.path.normcase(candidate)
|
||||
for candidate in (
|
||||
db_path_str,
|
||||
db_path_str + "-wal",
|
||||
db_path_str + "-shm",
|
||||
this_home,
|
||||
*((install_root,) if install_root else ()),
|
||||
)
|
||||
for candidate in (db_path_str, db_path_str + "-wal", db_path_str + "-shm")
|
||||
}
|
||||
own_prefixes = tuple(
|
||||
os.path.normcase(root) + os.sep
|
||||
for root in (this_home, *((install_root,) if install_root else ()))
|
||||
)
|
||||
other_home_seen = False
|
||||
for token in argv:
|
||||
if not isinstance(token, str):
|
||||
this_home_norm = os.path.normcase(this_home)
|
||||
root_norm = os.path.normcase(install_root) if install_root else None
|
||||
path_tokens = _argv_path_tokens(argv)
|
||||
|
||||
if any(normalized in sidecars for _, normalized in path_tokens):
|
||||
return False
|
||||
selection = _argv_home_selection(argv, this_home, install_root, our_profile)
|
||||
if selection == "ours":
|
||||
return False
|
||||
# A store whose home is not itself part of an install layout cannot be identified from the
|
||||
# install location, so argv[0] alone never dismisses a holder of it.
|
||||
argv0_locates_home = install_root is not None
|
||||
other_home_seen = selection == "other"
|
||||
for index, normalized in path_tokens:
|
||||
if _names_other_profile(normalized, install_root, our_profile):
|
||||
other_home_seen = True
|
||||
continue
|
||||
if token.startswith("/"):
|
||||
path_token = token
|
||||
elif token.startswith("-") and "=" in token:
|
||||
# ``--db=/abs/path``-style options carry a path value; anchor on
|
||||
# the text after '=' so normpath does not prepend the option.
|
||||
value = token.split("=", 1)[1]
|
||||
path_token = value if value.startswith("/") else None
|
||||
else:
|
||||
path_token = None
|
||||
if path_token is not None:
|
||||
normalized = os.path.normcase(os.path.normpath(path_token))
|
||||
if normalized in ours or normalized.startswith(own_prefixes):
|
||||
return False
|
||||
if "/.hermes" in normalized or normalized.endswith("/.hermes"):
|
||||
other_home_seen = True
|
||||
elif os.path.basename(normalized) in (
|
||||
"state.db",
|
||||
"state.db-wal",
|
||||
"state.db-shm",
|
||||
):
|
||||
other_home_seen = True
|
||||
if normalized == this_home_norm or normalized.startswith(this_home_norm + os.sep):
|
||||
return False
|
||||
if root_norm is not None and (
|
||||
normalized == root_norm or normalized.startswith(root_norm + os.sep)
|
||||
):
|
||||
continue # shared install root: neutral, every served profile lives under it
|
||||
if index == 0 and not argv0_locates_home:
|
||||
continue
|
||||
if "/.hermes" in normalized or normalized.endswith("/.hermes"):
|
||||
other_home_seen = True
|
||||
elif os.path.basename(normalized) in _STATE_DB_NAMES:
|
||||
other_home_seen = True
|
||||
return other_home_seen
|
||||
|
||||
|
||||
|
||||
@@ -401,25 +401,32 @@ def close_all_under(directory: str | Path) -> int:
|
||||
def other_generations_for_path(
|
||||
db_path: Path, *, exclude: Optional["SessionDB"] = None
|
||||
) -> List[str]:
|
||||
"""Describe every OTHER SessionDB generation THIS process holds for *db_path*.
|
||||
"""Describe every other LIVE SessionDB generation THIS process holds for *db_path*.
|
||||
|
||||
The registry is path-keyed, so it can answer the in-process half of "is this store quiet?"
|
||||
that a ``/proc`` descriptor scan structurally cannot: that scan skips our own pid, so it only
|
||||
ever proves other PROCESSES are away. Retired generations count — they stay open for their
|
||||
holders, and a VACUUM's TRUNCATE checkpoint retires the WAL generation underneath them.
|
||||
ever proves other PROCESSES are away.
|
||||
|
||||
RETIRED generations are deliberately not holders here. A generation is retired only after its
|
||||
file was replaced, which is exactly when ``SessionDB`` fences it: every write raises
|
||||
``StateDbReplacedError`` and the close-time checkpoint is disabled, so it is not the live writer
|
||||
this gate protects. It also leaves ``_retired`` only when its last holder releases, and a
|
||||
gateway handle does not release before shutdown — counting it made ONE inode replacement
|
||||
(recovery swap, backup restore, snapshot) skip auto-VACUUM for that path for the rest of the
|
||||
process lifetime, turning the unbounded growth this maintenance exists to bound into a
|
||||
permanent condition.
|
||||
"""
|
||||
try:
|
||||
path = Path(db_path).resolve()
|
||||
except OSError:
|
||||
path = Path(db_path)
|
||||
with _lock:
|
||||
candidates = [
|
||||
("live", generation) for generation in _generations.values()
|
||||
] + [("retired", generation) for generation in _retired.values()]
|
||||
return [
|
||||
f"in-process {kind} SessionDB generation (refcount {generation.refcount})"
|
||||
for kind, generation in candidates
|
||||
if generation.db is not exclude and generation.path == path
|
||||
f"in-process live SessionDB generation (refcount {generation.refcount})"
|
||||
for generation in _generations.values()
|
||||
if generation.db is not exclude
|
||||
and generation.path == path
|
||||
and not generation.retired
|
||||
]
|
||||
|
||||
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
flags (end/reopen/archive/pin/hide/read), model_config patching, listing and
|
||||
counting, delete cascades, and the auto-archive sweep."""
|
||||
|
||||
import glob
|
||||
import json
|
||||
import logging
|
||||
import re
|
||||
@@ -1502,7 +1503,9 @@ class SessionSessionsMixin:
|
||||
return
|
||||
targets = [sessions_dir / f"{session_id}{suffix}" for suffix in (".json", ".jsonl")]
|
||||
try:
|
||||
targets.extend(sessions_dir.glob(f"request_dump_{session_id}_*.json"))
|
||||
# glob.escape: a session id carrying ``[`` / ``?`` / ``*`` is a PATTERN otherwise, so the
|
||||
# dump sweep either matches nothing or matches another session's files.
|
||||
targets.extend(sessions_dir.glob(f"request_dump_{glob.escape(session_id)}_*.json"))
|
||||
except OSError:
|
||||
pass
|
||||
for p in targets:
|
||||
|
||||
@@ -59,6 +59,10 @@ def two_homes(tmp_path, monkeypatch):
|
||||
# Restore the import-time sentinel so an argless ``acquire()`` resolves through the scope.
|
||||
import hermes_state
|
||||
monkeypatch.setattr(hermes_state, "DEFAULT_DB_PATH", hermes_state._IMPORT_DEFAULT_DB_PATH)
|
||||
# Disabling the hermetic pin is only safe while the sentinel still resolves INSIDE the sandbox:
|
||||
# a resolution that escaped to the real home would have these tests writing the live store.
|
||||
resolved = Path(hermes_state._default_db_path())
|
||||
assert resolved.is_relative_to(tmp_path), f"unpinned store escaped the sandbox: {resolved}"
|
||||
return a, b
|
||||
|
||||
|
||||
@@ -174,6 +178,45 @@ def test_multiplexed_maintenance_tick_prunes_every_served_profile_store(two_home
|
||||
db.close()
|
||||
|
||||
|
||||
def test_prune_unlinks_transcripts_under_the_configured_sessions_dir(two_homes, tmp_path):
|
||||
"""``gateway.sessions_dir`` governs the LAUNCH profile's transcripts; others use their own home.
|
||||
|
||||
Hardcoding ``<home>/sessions`` made the prune unlink under a directory nothing writes to, so an
|
||||
override left every pruned session's ``.json``/``.jsonl``/``request_dump_*`` orphaned forever.
|
||||
"""
|
||||
from agent.secret_scope import set_multiplex_active
|
||||
from hermes_state import SessionDB
|
||||
|
||||
a, b = two_homes
|
||||
override = tmp_path / "custom-transcripts"
|
||||
override.mkdir()
|
||||
for home, transcripts in ((a, override), (b, b / "sessions")):
|
||||
(home / "config.yaml").write_text(
|
||||
"model:\n provider: nous\n"
|
||||
"sessions:\n"
|
||||
" auto_prune: true\n"
|
||||
" retention_days: 0\n"
|
||||
" min_interval_hours: 0\n"
|
||||
" vacuum_after_prune: false\n",
|
||||
encoding="utf-8")
|
||||
db = SessionDB(db_path=home / "state.db")
|
||||
db.create_session("old", "cli")
|
||||
db.end_session("old", "done")
|
||||
db.close()
|
||||
transcripts.mkdir(parents=True, exist_ok=True)
|
||||
(transcripts / "old.jsonl").write_text("{}\n", encoding="utf-8")
|
||||
|
||||
set_multiplex_active(True)
|
||||
try:
|
||||
_run_60_ticks(SimpleNamespace(config=SimpleNamespace(
|
||||
multiplex_profiles=True, sessions_dir=override)))
|
||||
finally:
|
||||
set_multiplex_active(False)
|
||||
|
||||
assert not (override / "old.jsonl").exists(), "launch profile's configured transcript survived"
|
||||
assert not (b / "sessions" / "old.jsonl").exists(), "profile b's transcript survived"
|
||||
|
||||
|
||||
def test_single_profile_sync_ticks_run_once_against_the_process_home(two_homes, monkeypatch):
|
||||
"""Control: a single-profile gateway (multiplex off) still runs each chore exactly once against
|
||||
the process home — the named profile directory on disk is not visited."""
|
||||
|
||||
@@ -1,9 +1,17 @@
|
||||
"""Automatic VACUUM is refused while THIS process holds another generation of the same store.
|
||||
"""Automatic VACUUM is refused under a LIVE in-process holder — and never under a retired one.
|
||||
|
||||
``foreign_state_db_holders`` skips ``os.getpid()``, so it can only ever prove other PROCESSES
|
||||
are away. Under the one-process-per-host multiplexer the dangerous holder is usually a sibling
|
||||
SessionDB generation inside this very process: the VACUUM's TRUNCATE checkpoint retires the WAL
|
||||
generation that handle is still bound to.
|
||||
``foreign_state_db_holders`` skips ``os.getpid()``, so it can only ever prove other PROCESSES are
|
||||
away. Under the one-process-per-host multiplexer the dangerous holder is a sibling SessionDB in
|
||||
this very process: the VACUUM's TRUNCATE checkpoint retires the WAL generation that handle is
|
||||
still bound to.
|
||||
|
||||
The mirror invariant matters just as much. A RETIRED generation (its file was replaced by a
|
||||
recovery swap, backup restore or snapshot) is already write-fenced with ``StateDbReplacedError``
|
||||
and its close-time checkpoint disabled, so it is not the writer this gate protects — and it leaves
|
||||
the registry only when its holder releases, which a gateway handle does not do before shutdown.
|
||||
Counting it made ONE inode replacement disable auto-VACUUM for that path for the rest of the
|
||||
process lifetime, turning the unbounded growth this maintenance exists to bound into a permanent
|
||||
condition.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
@@ -31,37 +39,68 @@ def _auto_maintenance(db):
|
||||
min_vacuum_freelist_ratio=-1.0)
|
||||
|
||||
|
||||
def test_auto_vacuum_skips_while_this_process_holds_another_generation(tmp_path):
|
||||
def _make_prunable(db, session_id):
|
||||
db.set_meta("last_auto_prune", "0")
|
||||
db.create_session(session_id, "cli")
|
||||
db.end_session(session_id, "done")
|
||||
|
||||
|
||||
def test_auto_vacuum_skips_while_a_live_sibling_holds_the_same_store(tmp_path):
|
||||
"""Invariant: a genuinely LIVE sibling SessionDB for this path defers the VACUUM."""
|
||||
from hermes_state import SessionDB
|
||||
|
||||
db_path = tmp_path / "state.db"
|
||||
_seed(db_path)
|
||||
|
||||
first = registry.acquire(db_path)
|
||||
live_sibling = registry.acquire(db_path) # e.g. the gateway's own handle
|
||||
maintainer = SessionDB(db_path=db_path) # a second handle running housekeeping
|
||||
try:
|
||||
# Snapshot restore / recovery swap shape: the file is replaced, so the next acquire
|
||||
# RETIRES `first` (still open, still held) and opens a fresh generation.
|
||||
_make_prunable(maintainer, "old2")
|
||||
result = _auto_maintenance(maintainer)
|
||||
assert result["pruned"] >= 1, result
|
||||
assert result["vacuumed"] is False, "VACUUM ran under a live in-process holder"
|
||||
assert result.get("vacuum_skipped_holders"), result
|
||||
finally:
|
||||
maintainer.close()
|
||||
registry.release(live_sibling)
|
||||
|
||||
# Control: the same call VACUUMs once that live sibling is gone.
|
||||
quiet = SessionDB(db_path=db_path)
|
||||
try:
|
||||
_make_prunable(quiet, "old3")
|
||||
again = _auto_maintenance(quiet)
|
||||
assert again["vacuumed"] is True, again
|
||||
assert "vacuum_skipped_holders" not in again, again
|
||||
finally:
|
||||
quiet.close()
|
||||
|
||||
|
||||
def test_auto_vacuum_is_not_starved_by_a_retired_write_fenced_generation(tmp_path):
|
||||
"""RED on base: one inode replacement stopped auto-VACUUM for the process lifetime.
|
||||
|
||||
The retired handle is never released (a gateway holds its SessionDB until shutdown), so on base
|
||||
every later round reported ``vacuum_skipped_holders`` and the store grew without bound.
|
||||
"""
|
||||
db_path = tmp_path / "state.db"
|
||||
_seed(db_path)
|
||||
|
||||
parked = registry.acquire(db_path)
|
||||
try:
|
||||
# Recovery swap / backup restore shape: the file is replaced, so the next acquire RETIRES
|
||||
# ``parked`` (still open, still held, already write-fenced) and opens a fresh generation.
|
||||
replacement = tmp_path / "replacement.db"
|
||||
shutil.copy2(db_path, replacement)
|
||||
replacement.replace(db_path)
|
||||
second = registry.acquire(db_path)
|
||||
current = registry.acquire(db_path)
|
||||
try:
|
||||
assert second is not first, "inode replacement did not mint a new generation"
|
||||
result = _auto_maintenance(second)
|
||||
assert result["pruned"] == 1
|
||||
assert result["vacuumed"] is False, "VACUUM ran under a live in-process holder"
|
||||
assert result.get("vacuum_skipped_holders")
|
||||
assert current is not parked, "inode replacement did not mint a new generation"
|
||||
for round_index in range(3): # consecutive rounds, fresh prunable rows each time
|
||||
_make_prunable(current, f"old-{round_index}")
|
||||
result = _auto_maintenance(current)
|
||||
assert result["pruned"] >= 1, (round_index, result)
|
||||
assert result["vacuumed"] is True, (round_index, result)
|
||||
assert "vacuum_skipped_holders" not in result, (round_index, result)
|
||||
finally:
|
||||
registry.release(second)
|
||||
registry.release(current)
|
||||
finally:
|
||||
registry.release(first)
|
||||
|
||||
# Control: the store is quiet once the other generation is released, so the same call VACUUMs.
|
||||
only = registry.acquire(db_path)
|
||||
try:
|
||||
only.set_meta("last_auto_prune", "0")
|
||||
only.create_session("old2", "cli")
|
||||
only.end_session("old2", "done")
|
||||
again = _auto_maintenance(only)
|
||||
assert again["vacuumed"] is True
|
||||
assert "vacuum_skipped_holders" not in again
|
||||
finally:
|
||||
registry.release(only)
|
||||
registry.release(parked)
|
||||
|
||||
@@ -112,7 +112,9 @@ class TestUninspectableHolderInstanceScope:
|
||||
def test_other_instance_argv_is_not_a_holder_of_our_db(self, tmp_path, monkeypatch):
|
||||
"""RED: fd dir unreadable + argv proves the process belongs to a
|
||||
DIFFERENT Hermes home → not a holder of our state.db."""
|
||||
db_path = tmp_path / "state.db"
|
||||
# A real ``.hermes`` home, as on the field host this test is drawn from: the install
|
||||
# location can only identify a home that is itself part of an install layout.
|
||||
db_path = tmp_path / ".hermes" / "state.db"
|
||||
_install_fake_proc(monkeypatch, tmp_path, unreadable_pids=(222,))
|
||||
_install_fake_argv(monkeypatch, {222: DEMO_HOME_ARGV})
|
||||
|
||||
@@ -155,3 +157,84 @@ class TestUninspectableHolderInstanceScope:
|
||||
_install_fake_proc(monkeypatch, db_path.parent, unreadable_pids=(222,))
|
||||
_install_fake_argv(monkeypatch, {222: other_install})
|
||||
assert hermes_state_holders.foreign_state_db_holders(db_path) == []
|
||||
|
||||
def test_shared_binary_plus_another_profile_selection_is_dismissed(self, tmp_path, monkeypatch):
|
||||
"""argv[0] is the SHARED install binary, so it cannot prove a hold of profile b's store.
|
||||
|
||||
Every hermes process on a normal host runs ``<root>/venv/bin/hermes``; counting that token
|
||||
as proof made ``hermes -p other chat -q`` an uninspectable holder of every OTHER profile's
|
||||
state.db, deferring its FTS rebuild and auto-VACUUM for as long as the sibling lived
|
||||
(#92401, inside a single install). The process's own ``-p``/``--profile`` selection decides.
|
||||
"""
|
||||
root = tmp_path / ".hermes"
|
||||
db_path = root / "profiles" / "b" / "state.db"
|
||||
db_path.parent.mkdir(parents=True)
|
||||
shared_binary = str(root / "venv" / "bin" / "hermes")
|
||||
|
||||
for argv, expected in (
|
||||
([shared_binary, "-p", "other", "chat", "-q"], []),
|
||||
([shared_binary, "--profile=other", "gateway", "run"], []),
|
||||
# Control: the sibling that DOES serve profile b is still a fail-closed holder.
|
||||
([shared_binary, "-p", "b", "gateway", "run"], [222]),
|
||||
# Control: no selection at all stays fail-closed (it may be the multiplexer).
|
||||
([shared_binary, "gateway", "run"], [222]),
|
||||
):
|
||||
_install_fake_proc(monkeypatch, db_path.parent, unreadable_pids=(222,))
|
||||
_install_fake_argv(monkeypatch, {222: argv})
|
||||
holders = hermes_state_holders.foreign_state_db_holders(db_path)
|
||||
assert [pid for pid, _ in holders] == expected, argv
|
||||
|
||||
def test_token_naming_another_profiles_store_is_dismissal_evidence(self, tmp_path, monkeypatch):
|
||||
"""A token positively naming ANOTHER profile's store is the strongest dismissal there is.
|
||||
|
||||
It sits under our install root, so a blanket own-prefix test read the strongest evidence of
|
||||
a different scope as proof of ours.
|
||||
"""
|
||||
root = tmp_path / ".hermes"
|
||||
db_path = root / "profiles" / "b" / "state.db"
|
||||
db_path.parent.mkdir(parents=True)
|
||||
other_store = root / "profiles" / "other" / "state.db"
|
||||
|
||||
for argv in (
|
||||
["hermes", f"--db={other_store}", "sessions", "optimize"],
|
||||
[str(root / "venv" / "bin" / "hermes"), "sessions", str(other_store)],
|
||||
):
|
||||
_install_fake_proc(monkeypatch, db_path.parent, unreadable_pids=(222,))
|
||||
_install_fake_argv(monkeypatch, {222: argv})
|
||||
assert hermes_state_holders.foreign_state_db_holders(db_path) == [], argv
|
||||
|
||||
def test_unrelated_install_under_a_non_hermes_profiles_tree_stays_dismissed(
|
||||
self, tmp_path, monkeypatch
|
||||
):
|
||||
"""``<X>/profiles/<n>/state.db`` does not make all of ``<X>`` ours.
|
||||
|
||||
A raw ``basename == "profiles"`` test promoted any such parent to the install root, so an
|
||||
unrelated Hermes install living under it was counted as a holder — the literal two-instance
|
||||
shape #92401 was filed about. The canonical ``named_profile_home`` predicate requires the
|
||||
parent to be a real Hermes home.
|
||||
"""
|
||||
work = tmp_path / "work"
|
||||
db_path = work / "profiles" / "b" / "state.db"
|
||||
db_path.parent.mkdir(parents=True)
|
||||
unrelated_home = work / "demo" / ".hermes"
|
||||
unrelated_home.mkdir(parents=True)
|
||||
|
||||
_install_fake_proc(monkeypatch, db_path.parent, unreadable_pids=(222,))
|
||||
_install_fake_argv(
|
||||
monkeypatch, {222: ["hermes", "--hermes-home", str(unrelated_home), "gateway", "run"]})
|
||||
assert hermes_state_holders.foreign_state_db_holders(db_path) == []
|
||||
|
||||
def test_custom_home_is_not_dismissed_by_the_install_location(self, tmp_path, monkeypatch):
|
||||
"""A store at a custom HERMES_HOME is SERVED BY the binary under ``~/.hermes``.
|
||||
|
||||
Dismissing on that argv[0] admitted auto-VACUUM and the FTS rebuild under the live
|
||||
multiplexer that holds the store. argv[0] locates the install, never the home.
|
||||
"""
|
||||
db_path = tmp_path / "custom-store" / "state.db"
|
||||
db_path.parent.mkdir(parents=True)
|
||||
|
||||
_install_fake_proc(monkeypatch, db_path.parent, unreadable_pids=(222,))
|
||||
_install_fake_argv(
|
||||
monkeypatch, {222: ["/home/u/.hermes/venv/bin/hermes", "gateway", "run"]})
|
||||
holders = hermes_state_holders.foreign_state_db_holders(db_path)
|
||||
assert [pid for pid, _ in holders] == [222]
|
||||
|
||||
23
tests/hermes_state/test_remove_session_files_glob_escape.py
Normal file
23
tests/hermes_state/test_remove_session_files_glob_escape.py
Normal file
@@ -0,0 +1,23 @@
|
||||
"""Pruned-session file removal is id-scoped even when the id carries glob metacharacters.
|
||||
|
||||
``request_dump_<id>_*.json`` was interpolated unescaped, so an id containing ``[``/``?``/``*`` was
|
||||
a PATTERN: its own dumps were left behind and another session's could be matched instead.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from hermes_state_sessions import SessionSessionsMixin
|
||||
|
||||
|
||||
def test_remove_session_files_escapes_glob_metacharacters(tmp_path):
|
||||
tricky, neighbour = "sess-[ab]-1", "sess-a-1"
|
||||
for session_id in (tricky, neighbour):
|
||||
(tmp_path / f"{session_id}.jsonl").write_text("{}\n", encoding="utf-8")
|
||||
(tmp_path / f"request_dump_{session_id}_0.json").write_text("{}", encoding="utf-8")
|
||||
|
||||
SessionSessionsMixin._remove_session_files(tmp_path, tricky)
|
||||
|
||||
assert not (tmp_path / f"{tricky}.jsonl").exists()
|
||||
assert not (tmp_path / f"request_dump_{tricky}_0.json").exists(), "own dump was not matched"
|
||||
assert (tmp_path / f"{neighbour}.jsonl").exists()
|
||||
assert (tmp_path / f"request_dump_{neighbour}_0.json").exists(), "neighbour's dump was removed"
|
||||
Reference in New Issue
Block a user