diff --git a/hermes_cli/session_recovery.py b/hermes_cli/session_recovery.py index 750ee33cb04d..f321b8403700 100644 --- a/hermes_cli/session_recovery.py +++ b/hermes_cli/session_recovery.py @@ -861,19 +861,39 @@ def _reconstruct_missing_sessions( if not orphaned: return result + title_sequence = 1 for session_id, first_timestamp, message_count in orphaned: started_at = float(first_timestamp) if first_timestamp is not None else 0.0 - destination.execute( - "INSERT OR IGNORE INTO sessions " + while True: + title = ( + f"[recovered {title_sequence}] " + "session metadata was unreadable" + ) + title_sequence += 1 + if ( + destination.execute( + "SELECT 1 FROM sessions WHERE title = ? LIMIT 1", + (title,), + ).fetchone() + is None + ): + break + + cursor = destination.execute( + "INSERT INTO sessions " "(id, source, started_at, title, message_count) " "VALUES (?, 'recovered', ?, ?, ?)", ( session_id, started_at, - "[recovered] session metadata was unreadable", + title, int(message_count), ), ) + if cursor.rowcount != 1: + raise sqlite3.IntegrityError( + f"failed to reconstruct missing session {session_id!r}" + ) result["sessions_reconstructed"] += 1 result["messages_retained"] += int(message_count) return result diff --git a/tests/hermes_cli/test_session_recovery.py b/tests/hermes_cli/test_session_recovery.py index 0cea4559948f..b477395a56ea 100644 --- a/tests/hermes_cli/test_session_recovery.py +++ b/tests/hermes_cli/test_session_recovery.py @@ -393,11 +393,21 @@ def test_partial_recovery_keeps_messages_when_sessions_are_unsalvageable( source = tmp_path / "sessions-destroyed.db" output = tmp_path / "sessions-destroyed-recovered.db" + messages_per_session = { + "doomed-session-a": 40, + "doomed-session-b": 35, + "doomed-session-c": 45, + } db = SessionDB(db_path=source) try: - db.create_session("doomed-session", "cli", cwd="/tmp/doomed") - for index in range(120): - db.append_message("doomed-session", "user", f"irreplaceable {index}") + for session_id, message_count in messages_per_session.items(): + db.create_session(session_id, "cli", cwd=f"/tmp/{session_id}") + for index in range(message_count): + db.append_message( + session_id, + "user", + f"irreplaceable {session_id} {index}", + ) finally: db.close() @@ -420,20 +430,26 @@ def test_partial_recovery_keeps_messages_when_sessions_are_unsalvageable( assert cleanup["messages_removed"] == 0, ( "salvaged messages were deleted for lack of a session row" ) - assert cleanup["sessions_reconstructed"] >= 1 + assert cleanup["sessions_reconstructed"] == len(messages_per_session) assert cleanup["messages_retained"] == 120 with sqlite3.connect(str(output)) as verify: - sessions = verify.execute("SELECT COUNT(*) FROM sessions").fetchone()[0] + recovered_sessions = verify.execute( + "SELECT id, source, title, message_count FROM sessions ORDER BY id" + ).fetchall() messages = verify.execute("SELECT COUNT(*) FROM messages").fetchone()[0] - placeholder_source = verify.execute( - "SELECT source FROM sessions LIMIT 1" - ).fetchone()[0] assert messages == 120, f"expected all 120 messages retained, got {messages}" - assert sessions >= 1 + assert len(recovered_sessions) == len(messages_per_session) - # A fabricated session must be identifiable as such. - assert placeholder_source == "recovered" + # Fabricated sessions must be identifiable and carry collision-safe titles. + assert {row[0] for row in recovered_sessions} == set(messages_per_session) + assert {row[1] for row in recovered_sessions} == {"recovered"} + recovered_titles = [str(row[2]) for row in recovered_sessions] + assert all(title.startswith("[recovered ") for title in recovered_titles) + assert len(set(recovered_titles)) == len(recovered_titles) + assert { + str(row[0]): int(row[3]) for row in recovered_sessions + } == messages_per_session # Retaining the data is still a lossy outcome and must say so. assert report["verification"]["loss_detected"] is True @@ -907,7 +923,7 @@ def test_cli_allow_partial_salvages_rows_across_a_corrupt_leaf( } -def test_partial_recovery_removes_messages_for_unreadable_sessions( +def test_partial_recovery_reconstructs_unreadable_sessions_without_message_loss( tmp_path: Path, ) -> None: source = tmp_path / "corrupt-sessions.db" @@ -935,19 +951,37 @@ def test_partial_recovery_removes_messages_for_unreadable_sessions( assert _sha256(source) == source_hash assert report["copy"]["sessions"]["status"] == "partial" assert report["copy"]["messages"]["status"] == "complete" - removed_messages = report["orphan_cleanup"]["messages_removed"] - assert removed_messages > 0 - assert report["orphan_cleanup"]["total_removed_or_relinked"] >= removed_messages + copied_sessions = int(report["copy"]["sessions"]["copied_rows"]) + expected_reconstructed = session_count - copied_sessions + cleanup = report["orphan_cleanup"] + assert expected_reconstructed > 1 + assert cleanup["sessions_reconstructed"] == expected_reconstructed + assert cleanup["messages_retained"] == expected_reconstructed + assert cleanup["messages_removed"] == 0 assert report["verification"]["foreign_key_check"] == [] + assert report["verification"]["integrity_check"] == ["ok"] + assert report["verification"]["table_counts"]["sessions"] == session_count + assert report["verification"]["table_counts"]["messages"] == session_count conn = sqlite3.connect(str(output)) try: - recovered_sessions = { - str(row[0]) for row in conn.execute("SELECT id FROM sessions") + recovered_sessions = conn.execute( + "SELECT id, source, title FROM sessions ORDER BY id" + ).fetchall() + recovered_ids = {str(row[0]) for row in recovered_sessions} + assert recovered_ids == { + f"partial-session-{session_number:04d}" + for session_number in range(session_count) } - assert "partial-session-0000" in recovered_sessions - assert f"partial-session-{session_count - 1:04d}" in recovered_sessions - assert 0 < len(recovered_sessions) < session_count + placeholder_rows = [ + row for row in recovered_sessions if str(row[1]) == "recovered" + ] + assert len(placeholder_rows) == expected_reconstructed + placeholder_titles = [str(row[2]) for row in placeholder_rows] + assert len(set(placeholder_titles)) == expected_reconstructed + assert all( + title.startswith("[recovered ") for title in placeholder_titles + ) assert ( conn.execute( "SELECT COUNT(*) FROM messages AS message " @@ -957,8 +991,9 @@ def test_partial_recovery_removes_messages_for_unreadable_sessions( ).fetchone()[0] == 0 ) - assert conn.execute("SELECT COUNT(*) FROM messages").fetchone()[0] == len( - recovered_sessions + assert ( + conn.execute("SELECT COUNT(*) FROM messages").fetchone()[0] + == session_count ) finally: conn.close()