From 3fdd52742bb06ef8b497ade35b87416a774407a0 Mon Sep 17 00:00:00 2001
From: catlog22 <catlog22@github.com>
Date: Mon, 29 Dec 2025 19:08:49 +0800
Subject: [PATCH] fix(storage): handle rollback failures in batch operations

Adds nested exception handling in add_files() and _migrate_fts_to_external()
to catch and log rollback failures. Uses exception chaining to preserve both
transaction and rollback errors, preventing silent database inconsistency.

Solution-ID: SOL-1735385400010
Issue-ID: ISS-1766921318981-10
Task-ID: T1
---
 .../src/codexlens/storage/sqlite_store.py     | 19 ++--
 codex-lens/tests/test_sqlite_store.py         | 87 +++++++++++++++++++
 2 files changed, 101 insertions(+), 5 deletions(-)

diff --git a/codex-lens/src/codexlens/storage/sqlite_store.py b/codex-lens/src/codexlens/storage/sqlite_store.py
index 369feef7..788029b6 100644
--- a/codex-lens/src/codexlens/storage/sqlite_store.py
+++ b/codex-lens/src/codexlens/storage/sqlite_store.py
@@ -330,8 +330,14 @@ class SQLiteStore:
                         )
                 
                 conn.commit()
-            except Exception:
-                conn.rollback()
+            except Exception as exc:
+                try:
+                    conn.rollback()
+                except Exception as rollback_exc:
+                    logger.error(
+                        "Rollback failed after add_files() error (%s): %s", exc, rollback_exc
+                    )
+                    raise exc.with_traceback(exc.__traceback__) from rollback_exc
                 raise
 
     def remove_file(self, path: str | Path) -> bool:
@@ -619,11 +625,14 @@ class SQLiteStore:
             conn.execute("INSERT INTO files_fts(files_fts) VALUES('rebuild')")
             conn.execute("DROP TABLE files_fts_legacy")
             conn.commit()
-        except sqlite3.DatabaseError:
+        except sqlite3.DatabaseError as exc:
             try:
                 conn.rollback()
-            except Exception:
-                pass
+            except Exception as rollback_exc:
+                logger.error(
+                    "Rollback failed during FTS schema migration (%s): %s", exc, rollback_exc
+                )
+                raise exc.with_traceback(exc.__traceback__) from rollback_exc
 
             try:
                 conn.execute("DROP TABLE IF EXISTS files_fts")
diff --git a/codex-lens/tests/test_sqlite_store.py b/codex-lens/tests/test_sqlite_store.py
index 3f4a037a..486a104c 100644
--- a/codex-lens/tests/test_sqlite_store.py
+++ b/codex-lens/tests/test_sqlite_store.py
@@ -3,12 +3,14 @@
 from __future__ import annotations
 
 import logging
+import sqlite3
 import threading
 import time
 from pathlib import Path
 
 import pytest
 
+from codexlens.entities import IndexedFile
 from codexlens.storage.sqlite_store import SQLiteStore
 
 
@@ -114,3 +116,88 @@ def test_cleanup_robustness(
     finally:
         store.close()
 
+
+def test_add_files_rollback_preserves_original_exception(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
+    """add_files should re-raise the transaction error when rollback succeeds."""
+    monkeypatch.setattr(SQLiteStore, "CLEANUP_INTERVAL", 0)
+    store = SQLiteStore(tmp_path / "add_files_ok.db")
+    store.initialize()
+
+    real_conn = store._get_connection()
+
+    class FailingConnection:
+        def __init__(self, conn: sqlite3.Connection) -> None:
+            self._conn = conn
+            self.rollback_calls = 0
+
+        def execute(self, sql: str, params: tuple = ()):
+            if "INSERT INTO files" in sql:
+                raise sqlite3.OperationalError("boom")
+            return self._conn.execute(sql, params)
+
+        def executemany(self, sql: str, seq):
+            return self._conn.executemany(sql, seq)
+
+        def commit(self) -> None:
+            self._conn.commit()
+
+        def rollback(self) -> None:
+            self.rollback_calls += 1
+            self._conn.rollback()
+
+    wrapped = FailingConnection(real_conn)
+    monkeypatch.setattr(store, "_get_connection", lambda: wrapped)
+
+    indexed_file = IndexedFile(path=str(tmp_path / "a.py"), language="python", symbols=[])
+
+    try:
+        with pytest.raises(sqlite3.OperationalError, match="boom"):
+            store.add_files([(indexed_file, "# content")])
+        assert wrapped.rollback_calls == 1
+    finally:
+        store.close()
+
+
+def test_add_files_rollback_failure_is_chained(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture
+) -> None:
+    """Rollback failures should be logged and chained as the cause."""
+    monkeypatch.setattr(SQLiteStore, "CLEANUP_INTERVAL", 0)
+    caplog.set_level(logging.ERROR, logger="codexlens.storage.sqlite_store")
+
+    store = SQLiteStore(tmp_path / "add_files_rollback_fail.db")
+    store.initialize()
+    real_conn = store._get_connection()
+
+    class FailingRollbackConnection:
+        def __init__(self, conn: sqlite3.Connection) -> None:
+            self._conn = conn
+
+        def execute(self, sql: str, params: tuple = ()):
+            if "INSERT INTO files" in sql:
+                raise sqlite3.OperationalError("boom")
+            return self._conn.execute(sql, params)
+
+        def executemany(self, sql: str, seq):
+            return self._conn.executemany(sql, seq)
+
+        def commit(self) -> None:
+            self._conn.commit()
+
+        def rollback(self) -> None:
+            raise sqlite3.OperationalError("rollback boom")
+
+    monkeypatch.setattr(store, "_get_connection", lambda: FailingRollbackConnection(real_conn))
+    indexed_file = IndexedFile(path=str(tmp_path / "b.py"), language="python", symbols=[])
+
+    try:
+        with pytest.raises(sqlite3.OperationalError) as exc:
+            store.add_files([(indexed_file, "# content")])
+
+        assert exc.value.__cause__ is not None
+        assert isinstance(exc.value.__cause__, sqlite3.OperationalError)
+        assert "rollback boom" in str(exc.value.__cause__)
+        assert "Rollback failed after add_files() error" in caplog.text
+        assert "boom" in caplog.text
+    finally:
+        store.close()