fix(state): orphan children instead of cascade-deleting in prune/delete (#6513)
prune_sessions and delete_session only handled direct children when satisfying the parent_session_id FK constraint. Multi-level chains (A -> B -> C) caused IntegrityError because deleting B while C still referenced it was blocked by the FK. Fix: NULL out parent_session_id for any session whose parent is about to be deleted. This orphans children instead of cascade-deleting them, which also respects the prune retention window — newer child sessions are no longer deleted just because an ancestor is old. Reported by Aaryan2304 in PR #6463.
This commit is contained in:
@@ -1235,10 +1235,10 @@ class SessionDB:
|
||||
self._execute_write(_do)
|
||||
|
||||
def delete_session(self, session_id: str) -> bool:
|
||||
"""Delete a session, its child sessions, and all their messages.
|
||||
"""Delete a session and all its messages.
|
||||
|
||||
Child sessions (subagent runs, compression continuations) are deleted
|
||||
first to satisfy the ``parent_session_id`` foreign key constraint.
|
||||
Child sessions are orphaned (parent_session_id set to NULL) rather
|
||||
than cascade-deleted, so they remain accessible independently.
|
||||
Returns True if the session was found and deleted.
|
||||
"""
|
||||
def _do(conn):
|
||||
@@ -1247,15 +1247,12 @@ class SessionDB:
|
||||
)
|
||||
if cursor.fetchone()[0] == 0:
|
||||
return False
|
||||
# Delete child sessions first (FK constraint)
|
||||
child_ids = [r[0] for r in conn.execute(
|
||||
"SELECT id FROM sessions WHERE parent_session_id = ?",
|
||||
# Orphan child sessions so FK constraint is satisfied
|
||||
conn.execute(
|
||||
"UPDATE sessions SET parent_session_id = NULL "
|
||||
"WHERE parent_session_id = ?",
|
||||
(session_id,),
|
||||
).fetchall()]
|
||||
for cid in child_ids:
|
||||
conn.execute("DELETE FROM messages WHERE session_id = ?", (cid,))
|
||||
conn.execute("DELETE FROM sessions WHERE id = ?", (cid,))
|
||||
# Delete the session itself
|
||||
)
|
||||
conn.execute("DELETE FROM messages WHERE session_id = ?", (session_id,))
|
||||
conn.execute("DELETE FROM sessions WHERE id = ?", (session_id,))
|
||||
return True
|
||||
@@ -1264,9 +1261,9 @@ class SessionDB:
|
||||
def prune_sessions(self, older_than_days: int = 90, source: str = None) -> int:
|
||||
"""Delete sessions older than N days. Returns count of deleted sessions.
|
||||
|
||||
Only prunes ended sessions (not active ones). Child sessions whose
|
||||
parents are being pruned are deleted first to satisfy the
|
||||
``parent_session_id`` foreign key constraint.
|
||||
Only prunes ended sessions (not active ones). Child sessions outside
|
||||
the prune window are orphaned (parent_session_id set to NULL) rather
|
||||
than cascade-deleted.
|
||||
"""
|
||||
cutoff = time.time() - (older_than_days * 86400)
|
||||
|
||||
@@ -1284,17 +1281,16 @@ class SessionDB:
|
||||
)
|
||||
session_ids = set(row["id"] for row in cursor.fetchall())
|
||||
|
||||
# Delete children first whose parents are in the prune set
|
||||
# (avoids FK constraint errors)
|
||||
for sid in list(session_ids):
|
||||
child_ids = [r[0] for r in conn.execute(
|
||||
"SELECT id FROM sessions WHERE parent_session_id = ?",
|
||||
(sid,),
|
||||
).fetchall()]
|
||||
for cid in child_ids:
|
||||
conn.execute("DELETE FROM messages WHERE session_id = ?", (cid,))
|
||||
conn.execute("DELETE FROM sessions WHERE id = ?", (cid,))
|
||||
session_ids.discard(cid) # don't double-delete
|
||||
if not session_ids:
|
||||
return 0
|
||||
|
||||
# Orphan any sessions whose parent is about to be deleted
|
||||
placeholders = ",".join("?" * len(session_ids))
|
||||
conn.execute(
|
||||
f"UPDATE sessions SET parent_session_id = NULL "
|
||||
f"WHERE parent_session_id IN ({placeholders})",
|
||||
list(session_ids),
|
||||
)
|
||||
|
||||
for sid in session_ids:
|
||||
conn.execute("DELETE FROM messages WHERE session_id = ?", (sid,))
|
||||
|
||||
@@ -663,6 +663,84 @@ class TestPruneSessions:
|
||||
assert db.get_session("old_cli") is None
|
||||
assert db.get_session("old_tg") is not None
|
||||
|
||||
def test_prune_with_multilevel_chain(self, db):
|
||||
"""Pruning old sessions orphans newer children instead of crashing on FK."""
|
||||
old_ts = time.time() - 200 * 86400
|
||||
recent_ts = time.time() - 10 * 86400
|
||||
|
||||
# Chain: A (old) -> B (old) -> C (recent) -> D (recent)
|
||||
db.create_session(session_id="A", source="cli")
|
||||
db.end_session("A", end_reason="compressed")
|
||||
db.create_session(session_id="B", source="cli", parent_session_id="A")
|
||||
db.end_session("B", end_reason="compressed")
|
||||
db.create_session(session_id="C", source="cli", parent_session_id="B")
|
||||
db.end_session("C", end_reason="compressed")
|
||||
db.create_session(session_id="D", source="cli", parent_session_id="C")
|
||||
db.end_session("D", end_reason="done")
|
||||
|
||||
# Backdate A and B to be old; C and D stay recent
|
||||
for sid, ts in [("A", old_ts), ("B", old_ts), ("C", recent_ts), ("D", recent_ts)]:
|
||||
db._conn.execute(
|
||||
"UPDATE sessions SET started_at = ? WHERE id = ?", (ts, sid)
|
||||
)
|
||||
db._conn.commit()
|
||||
|
||||
# Should not raise IntegrityError
|
||||
pruned = db.prune_sessions(older_than_days=90)
|
||||
assert pruned == 2 # only A and B
|
||||
assert db.get_session("A") is None
|
||||
assert db.get_session("B") is None
|
||||
# C and D survive, C is orphaned (parent_session_id NULL)
|
||||
c = db.get_session("C")
|
||||
assert c is not None
|
||||
assert c["parent_session_id"] is None
|
||||
d = db.get_session("D")
|
||||
assert d is not None
|
||||
assert d["parent_session_id"] == "C"
|
||||
|
||||
def test_prune_entire_old_chain(self, db):
|
||||
"""All sessions in a chain are old — entire chain is pruned."""
|
||||
old_ts = time.time() - 200 * 86400
|
||||
|
||||
db.create_session(session_id="X", source="cli")
|
||||
db.end_session("X", end_reason="compressed")
|
||||
db.create_session(session_id="Y", source="cli", parent_session_id="X")
|
||||
db.end_session("Y", end_reason="compressed")
|
||||
db.create_session(session_id="Z", source="cli", parent_session_id="Y")
|
||||
db.end_session("Z", end_reason="done")
|
||||
|
||||
for sid in ("X", "Y", "Z"):
|
||||
db._conn.execute(
|
||||
"UPDATE sessions SET started_at = ? WHERE id = ?", (old_ts, sid)
|
||||
)
|
||||
db._conn.commit()
|
||||
|
||||
pruned = db.prune_sessions(older_than_days=90)
|
||||
assert pruned == 3
|
||||
for sid in ("X", "Y", "Z"):
|
||||
assert db.get_session(sid) is None
|
||||
|
||||
|
||||
class TestDeleteSessionOrphansChildren:
|
||||
def test_delete_orphans_children(self, db):
|
||||
"""Deleting a parent session orphans its children."""
|
||||
db.create_session(session_id="parent", source="cli")
|
||||
db.create_session(session_id="child", source="cli", parent_session_id="parent")
|
||||
db.create_session(session_id="grandchild", source="cli", parent_session_id="child")
|
||||
|
||||
# Should not raise IntegrityError
|
||||
result = db.delete_session("parent")
|
||||
assert result is True
|
||||
assert db.get_session("parent") is None
|
||||
# Child is orphaned, not deleted
|
||||
child = db.get_session("child")
|
||||
assert child is not None
|
||||
assert child["parent_session_id"] is None
|
||||
# Grandchild is untouched
|
||||
grandchild = db.get_session("grandchild")
|
||||
assert grandchild is not None
|
||||
assert grandchild["parent_session_id"] == "child"
|
||||
|
||||
|
||||
# =========================================================================
|
||||
# Schema and WAL mode
|
||||
|
||||
Reference in New Issue
Block a user