fix(sessions): tip-only resume guard on the CLI mid-setup path; fail open on guard errors
The mid-setup CLI resume path loads only the tip session's rows, so gate it with a tip-only count instead of the full-lineage count (which over-rejected heavily-compressed sessions). Transient guard failures (locked DB, adaptor stores) now log and proceed instead of blocking resume with a new error.
This commit is contained in:
parent
2607dc9a85
commit
5b4b9bbf77
|
|
@ -407,7 +407,10 @@ class CLIAgentSetupMixin:
|
|||
prior_resume_error = getattr(self, "_resume_history_error", None)
|
||||
if prior_resume_error:
|
||||
return False
|
||||
resume_limit_error = self._resume_history_limit_error()
|
||||
# This path loads only the TIP session's rows (no ancestors),
|
||||
# so guard with a tip-only count — the full-lineage count would
|
||||
# over-reject heavily-compressed sessions with a small tip.
|
||||
resume_limit_error = self._resume_history_limit_error(tip_only=True)
|
||||
if resume_limit_error:
|
||||
self._resume_history_error = resume_limit_error
|
||||
if _quiet_mode:
|
||||
|
|
@ -586,22 +589,48 @@ class CLIAgentSetupMixin:
|
|||
console.print(line)
|
||||
return False
|
||||
|
||||
def _resume_history_limit_error(self):
|
||||
"""Return a safe-resume error without materializing transcript rows."""
|
||||
def _resume_history_limit_error(self, tip_only: bool = False):
|
||||
"""Return a safe-resume error without materializing transcript rows.
|
||||
|
||||
``tip_only`` matches call sites that load only the tip session's rows
|
||||
(``get_messages_as_conversation`` without ancestors) — counting the
|
||||
full lineage there would over-reject heavily-compressed sessions
|
||||
whose tip is small. Generic guard failures fail OPEN (resume
|
||||
proceeds) — only a genuine over-limit result blocks.
|
||||
"""
|
||||
if not self._session_db:
|
||||
return None
|
||||
safety_check = getattr(self._session_db, "assert_resume_safe", None)
|
||||
if not callable(safety_check):
|
||||
return None
|
||||
try:
|
||||
safety_check(self.session_id)
|
||||
except Exception as exc:
|
||||
from hermes_state import SessionResumeTooLargeError
|
||||
from hermes_state import (
|
||||
SessionExportTooLargeError,
|
||||
SessionResumeTooLargeError,
|
||||
resolved_max_resume_messages,
|
||||
)
|
||||
|
||||
if isinstance(exc, SessionResumeTooLargeError):
|
||||
return str(exc)
|
||||
logger.warning("Resume safety check failed for %s: %s", self.session_id, exc)
|
||||
return f"resume safety check failed: {exc}"
|
||||
try:
|
||||
if tip_only:
|
||||
tip_check = getattr(self._session_db, "assert_export_safe", None)
|
||||
if not callable(tip_check):
|
||||
return None
|
||||
limit = resolved_max_resume_messages()
|
||||
if limit <= 0:
|
||||
return None
|
||||
try:
|
||||
tip_check(self.session_id, max_messages=limit)
|
||||
except SessionExportTooLargeError as exc:
|
||||
raise SessionResumeTooLargeError(exc.message_count, limit) from exc
|
||||
else:
|
||||
safety_check = getattr(self._session_db, "assert_resume_safe", None)
|
||||
if not callable(safety_check):
|
||||
return None
|
||||
safety_check(self.session_id)
|
||||
except SessionResumeTooLargeError as exc:
|
||||
return str(exc)
|
||||
except Exception as exc:
|
||||
logger.warning(
|
||||
"Resume safety check failed for %s (proceeding without guard): %s",
|
||||
self.session_id, exc,
|
||||
)
|
||||
return None
|
||||
return None
|
||||
|
||||
def _preload_resumed_session(self) -> bool:
|
||||
|
|
|
|||
|
|
@ -382,6 +382,49 @@ def test_session_resume_rejects_runaway_transcript_before_history_load(
|
|||
assert "safe resume limit is 20000" in response["error"]["message"]
|
||||
|
||||
|
||||
def test_session_resume_guard_failure_fails_open(server, monkeypatch):
|
||||
"""A transient guard error must not block resume (fail open, log only)."""
|
||||
reopened = []
|
||||
|
||||
class _DB:
|
||||
def get_session(self, sid):
|
||||
return {"id": sid}
|
||||
|
||||
def get_session_by_title(self, _title):
|
||||
return None
|
||||
|
||||
def resolve_resume_session_id(self, sid):
|
||||
return sid
|
||||
|
||||
def assert_resume_safe(self, _sid):
|
||||
raise RuntimeError("database is locked")
|
||||
|
||||
def reopen_session(self, sid):
|
||||
reopened.append(sid)
|
||||
return True
|
||||
|
||||
monkeypatch.setattr(server, "_get_db", lambda: _DB())
|
||||
|
||||
response = server.handle_request(
|
||||
{
|
||||
"id": "r-open",
|
||||
"method": "session.resume",
|
||||
"params": {
|
||||
"session_id": "transient-guard-session",
|
||||
"omit_messages": True,
|
||||
},
|
||||
}
|
||||
)
|
||||
|
||||
# The guard must not block: no 4130, and any downstream failure must not
|
||||
# be the guard's own "resume safety check failed" error. Reopen being
|
||||
# attempted proves execution moved past the guard.
|
||||
err = response.get("error") or {}
|
||||
assert err.get("code") != 4130
|
||||
assert "resume safety check failed" not in str(err.get("message", ""))
|
||||
assert reopened == ["transient-guard-session"]
|
||||
|
||||
|
||||
def test_enforce_session_cap_evicts_oldest_detached_only(server, monkeypatch):
|
||||
"""The LRU cap frees the least-recently-active DETACHED sessions when over
|
||||
the limit, and never a live-transport / running / mid-build one."""
|
||||
|
|
|
|||
|
|
@ -401,7 +401,13 @@ def _(rid, params: dict) -> dict:
|
|||
except SessionResumeTooLargeError as exc:
|
||||
return _err(rid, 4130, str(exc))
|
||||
except Exception as exc:
|
||||
return _err(rid, 5000, f"resume safety check failed: {exc}")
|
||||
# Fail OPEN: a transient guard failure (locked DB, schema skew on
|
||||
# an adaptor store) must not turn the safety check into a new way
|
||||
# to lose access to a session. Only a genuine over-limit blocks.
|
||||
logger.warning(
|
||||
"resume safety check failed for %s (proceeding without guard): %s",
|
||||
target, exc,
|
||||
)
|
||||
|
||||
profile_resume_cwd = str(found.get("cwd") or "").strip() or _profile_configured_cwd(
|
||||
profile_home
|
||||
|
|
|
|||
Loading…
Reference in New Issue