fix(gateway): compensate half-written seeded branches + observable failures
Review follow-up on #93959: 1. Partial-failure window: if the row commits but the transcript copy or title write fails, the durable-but-empty child defeated the lazy first-prompt fallback (_ensure_session_db_row is INSERT OR IGNORE), so the renderer fail-latched on a transcript-less session again. The seed block now compensates: delete just this child so the lazy path can retry cleanly. Disk-full is exempt — deleting data on a full disk makes things worse. 2. Silent degradation: the best-effort catch now logs at WARNING with exc_info instead of DEBUG, so a regression in this user-facing path is observable without enabling debug logs. Tests: compensation deletes the half-written row and preserves pending_title; disk-full keeps the row and surfaces the WARNING.
This commit is contained in:
@@ -15555,6 +15555,143 @@ def test_session_create_branch_seed_failure_does_not_break_create(monkeypatch):
|
||||
server._sessions.pop(resp["result"]["stored_session_id"], None)
|
||||
|
||||
|
||||
def test_session_create_seed_failure_after_row_compensates(monkeypatch):
|
||||
"""Partial-failure compensation (#93959 review): if the row commits but
|
||||
the transcript copy fails, the just-created child is DELETED so the lazy
|
||||
first-prompt fallback can retry cleanly. Without this, a durable empty
|
||||
row defeats _ensure_session_db_row's INSERT OR IGNORE and the renderer
|
||||
fail-latches on a transcript-less session again."""
|
||||
|
||||
class _FakeAgent:
|
||||
def __init__(self):
|
||||
self.model = "test-model"
|
||||
|
||||
seen: dict = {}
|
||||
|
||||
class _FakeDB:
|
||||
def get_session_title(self, key):
|
||||
return "Parent"
|
||||
|
||||
def get_next_title_in_lineage(self, current):
|
||||
return f"{current} #2"
|
||||
|
||||
def create_session(self, key, **kwargs):
|
||||
seen["created"] = key
|
||||
|
||||
def append_messages_batch(self, session_id, messages, **kwargs):
|
||||
raise RuntimeError("transcript write failed")
|
||||
|
||||
def delete_session(self, session_id):
|
||||
seen["deleted"] = session_id
|
||||
return True
|
||||
|
||||
monkeypatch.setattr(server, "_get_db", lambda: _FakeDB())
|
||||
monkeypatch.setattr(server, "_make_agent", lambda sid, key, session_db=None, **_kw: _FakeAgent())
|
||||
monkeypatch.setattr(server, "_SlashWorker", lambda *a, **k: None)
|
||||
monkeypatch.setattr(server, "_session_info", lambda _a, *a2: {"model": "x"})
|
||||
monkeypatch.setattr(server, "_probe_credentials", lambda _a: None)
|
||||
monkeypatch.setattr(server, "_wire_callbacks", lambda _sid: None)
|
||||
monkeypatch.setattr(server, "_emit", lambda *a, **kw: None)
|
||||
|
||||
import tools.approval as _approval
|
||||
|
||||
monkeypatch.setattr(_approval, "register_gateway_notify", lambda key, cb: None)
|
||||
monkeypatch.setattr(_approval, "load_permanent_allowlist", lambda: None)
|
||||
|
||||
resp = server.handle_request(
|
||||
{
|
||||
"id": "1",
|
||||
"method": "session.create",
|
||||
"params": {
|
||||
"source": "desktop",
|
||||
"parent_session_id": "parent-1",
|
||||
"title": "My Branch",
|
||||
"messages": [{"role": "user", "content": "seed"}],
|
||||
},
|
||||
}
|
||||
)
|
||||
|
||||
assert "result" in resp
|
||||
key = resp["result"]["stored_session_id"]
|
||||
# The half-written child was rolled back — no durable empty row left to
|
||||
# shadow the lazy seed path.
|
||||
assert seen.get("deleted") == key
|
||||
# pending_title survived: it still lands via the lazy post-turn apply.
|
||||
runtime_sid = resp["result"]["session_id"]
|
||||
assert server._sessions[runtime_sid]["pending_title"] == "My Branch"
|
||||
|
||||
server._sessions.pop(runtime_sid, None)
|
||||
|
||||
|
||||
def test_session_create_seed_disk_full_keeps_row_for_retry(monkeypatch):
|
||||
"""Disk-full is NOT compensated: the row stays (deleting data on a full
|
||||
disk can make things worse), create still succeeds, and the failure is
|
||||
observable at warning level (#93959 review)."""
|
||||
|
||||
import logging as _logging
|
||||
|
||||
class _FakeAgent:
|
||||
def __init__(self):
|
||||
self.model = "test-model"
|
||||
|
||||
class _FakeDB:
|
||||
def get_session_title(self, key):
|
||||
return "Parent"
|
||||
|
||||
def get_next_title_in_lineage(self, current):
|
||||
return f"{current} #2"
|
||||
|
||||
def create_session(self, key, **kwargs):
|
||||
pass
|
||||
|
||||
def append_messages_batch(self, session_id, messages, **kwargs):
|
||||
raise OSError(28, "No space left on device")
|
||||
|
||||
monkeypatch.setattr(server, "_get_db", lambda: _FakeDB())
|
||||
monkeypatch.setattr(server, "_make_agent", lambda sid, key, session_db=None, **_kw: _FakeAgent())
|
||||
monkeypatch.setattr(server, "_SlashWorker", lambda *a, **k: None)
|
||||
monkeypatch.setattr(server, "_session_info", lambda _a, *a2: {"model": "x"})
|
||||
monkeypatch.setattr(server, "_probe_credentials", lambda _a: None)
|
||||
monkeypatch.setattr(server, "_wire_callbacks", lambda _sid: None)
|
||||
monkeypatch.setattr(server, "_emit", lambda *a, **kw: None)
|
||||
|
||||
import tools.approval as _approval
|
||||
|
||||
monkeypatch.setattr(_approval, "register_gateway_notify", lambda key, cb: None)
|
||||
monkeypatch.setattr(_approval, "load_permanent_allowlist", lambda: None)
|
||||
|
||||
records: list = []
|
||||
|
||||
class _Capture(_logging.Handler):
|
||||
def emit(self, record):
|
||||
records.append(record)
|
||||
|
||||
handler = _Capture(level=_logging.WARNING)
|
||||
root = _logging.getLogger()
|
||||
root.addHandler(handler)
|
||||
try:
|
||||
resp = server.handle_request(
|
||||
{
|
||||
"id": "1",
|
||||
"method": "session.create",
|
||||
"params": {
|
||||
"source": "desktop",
|
||||
"parent_session_id": "parent-1",
|
||||
"messages": [{"role": "user", "content": "seed"}],
|
||||
},
|
||||
}
|
||||
)
|
||||
finally:
|
||||
root.removeHandler(handler)
|
||||
|
||||
assert "result" in resp
|
||||
# The failure surfaced at WARNING (observable), not buried at debug.
|
||||
warnings = [r for r in records if r.levelno >= _logging.WARNING]
|
||||
assert any("seeded-branch persistence failed" in r.getMessage() for r in warnings)
|
||||
|
||||
server._sessions.pop(resp["result"]["stored_session_id"], None)
|
||||
|
||||
|
||||
def test_session_create_without_parent_still_defers_row(monkeypatch):
|
||||
"""Plain drafts keep the lazy-row contract: no parent + no explicit branch
|
||||
intent means no eager persistence (the original draft-hygiene invariant)."""
|
||||
|
||||
@@ -152,7 +152,15 @@ def _(rid, params: dict) -> dict:
|
||||
Path(profile_home).name if profile_home else None
|
||||
),
|
||||
)
|
||||
if history:
|
||||
# Compensation guard (#93959 review): if the transcript
|
||||
# copy or title write fails AFTER the row committed, the
|
||||
# durable-but-empty row would defeat the lazy first-prompt
|
||||
# fallback (_ensure_session_db_row is INSERT OR IGNORE —
|
||||
# the row exists, so the seed never lands and the renderer
|
||||
# fail-latches on a "transcript-less" session again).
|
||||
# Roll back just this child so the seed path can retry
|
||||
# cleanly on first submit.
|
||||
try:
|
||||
db.append_messages_batch(
|
||||
key,
|
||||
[
|
||||
@@ -161,13 +169,32 @@ def _(rid, params: dict) -> dict:
|
||||
],
|
||||
chunk_rows=500,
|
||||
)
|
||||
db.set_session_title(key, branch_title)
|
||||
db.set_session_title(key, branch_title)
|
||||
except Exception as exc:
|
||||
from hermes_state import is_disk_full_error
|
||||
|
||||
if is_disk_full_error(exc):
|
||||
raise
|
||||
try:
|
||||
db.delete_session(key)
|
||||
except Exception:
|
||||
logger.debug(
|
||||
"branch seed compensation delete failed for %s",
|
||||
key,
|
||||
exc_info=True,
|
||||
)
|
||||
raise
|
||||
_sessions[sid]["pending_title"] = None
|
||||
except Exception:
|
||||
# Persistence is best-effort here: a failed write must not break
|
||||
# session.create itself — the lazy first-prompt path remains as the
|
||||
# fallback, exactly as for plain drafts.
|
||||
logger.debug("branch seed persistence failed for %s", key, exc_info=True)
|
||||
logger.warning(
|
||||
"seeded-branch persistence failed for %s; falling back to "
|
||||
"lazy row creation",
|
||||
key,
|
||||
exc_info=True,
|
||||
)
|
||||
|
||||
# Return the lightweight session immediately so Ink can paint the composer
|
||||
# + skeleton panel, then build the real AIAgent just after this response is
|
||||
|
||||
Reference in New Issue
Block a user