Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion hermes_cli/kanban_db.py
Original file line number Diff line number Diff line change
Expand Up @@ -1473,12 +1473,22 @@ def write_txn(conn: sqlite3.Connection):
Use for any multi-statement write (creating a task + link, claiming a
task + recording an event, etc.). A claim CAS inside this context is
atomic -- at most one concurrent writer can succeed.

The explicit ROLLBACK on exception is wrapped in try/except so that
a SQLite auto-rollback (which leaves no active transaction) does not
shadow the original exception with a spurious rollback error.
"""
conn.execute("BEGIN IMMEDIATE")
try:
yield conn
except Exception:
conn.execute("ROLLBACK")
try:
conn.execute("ROLLBACK")
except sqlite3.OperationalError:
# SQLite has already auto-rolled-back the transaction (typical
# under EIO, lock contention, or corruption). Nothing to undo;
# do not let this secondary failure shadow the real one.
pass
raise
else:
conn.execute("COMMIT")
Expand Down
1 change: 1 addition & 0 deletions scripts/release.py
Original file line number Diff line number Diff line change
Expand Up @@ -1284,6 +1284,7 @@
"erik.engervall@gmail.com": "erikengervall", # PR #28774 (firecrawl integration tag)
"egilewski@egilewski.com": "egilewski", # PR #30432 (MEDIA path traversal fix, GHSA-jmf9-9729-7pp8)
"edison@mcclean.codes": "McClean-Edison", # PR #29817 (register_auxiliary_task plugin API)
"steveonjava@gmail.com": "steveonjava", # PR #31310 (write_txn rollback guard)
"zhangsamuel12@gmail.com": "SamuelZ12", # PR #7480 (show recap after in-session resume)
"490408354@qq.com": "daizhonggeng", # PR #9020 (numbered /resume selection)
}
Expand Down
58 changes: 58 additions & 0 deletions tests/hermes_cli/test_kanban_db.py
Original file line number Diff line number Diff line change
Expand Up @@ -3339,3 +3339,61 @@ def test_maybe_emit_scratch_tip_skips_non_scratch_workspaces(kanban_home, caplog
).fetchall()
assert "tip_scratch_workspace" not in [e["kind"] for e in events]


# write_txn — rollback handler must not mask the original exception
# ---------------------------------------------------------------------------


def test_write_txn_preserves_original_exception_when_rollback_fails(kanban_home):
"""When a write inside write_txn raises an OperationalError that SQLite
has already auto-rolled-back (e.g. ``disk I/O error``,
``database is locked``, ``database disk image is malformed``), the
explicit ROLLBACK in ``write_txn.__exit__`` itself raises
``cannot rollback - no transaction is active``. The original cause
must NOT be masked by the secondary rollback failure — operators rely
on the original cause to diagnose the underlying issue.
"""

class FailingConnWrapper:
"""Delegate to a real connection, simulating an EIO during an INSERT
that SQLite has already auto-rolled-back."""

def __init__(self, real):
self._real = real
self._fail_armed = True

def execute(self, sql, *args, **kwargs):
if (
self._fail_armed
and sql.lstrip().upper().startswith("INSERT")
and "task_events" in sql.lower()
):
self._fail_armed = False # one-shot
# Simulate SQLite auto-rolling back the transaction by
# issuing a real ROLLBACK now. After this, BEGIN IMMEDIATE
# is no longer active and an explicit ROLLBACK would error.
try:
self._real.execute("ROLLBACK")
except sqlite3.OperationalError:
pass
raise sqlite3.OperationalError("disk I/O error")
return self._real.execute(sql, *args, **kwargs)

def __getattr__(self, name):
return getattr(self._real, name)

with kb.connect() as conn:
wrapper = FailingConnWrapper(conn)
with pytest.raises(sqlite3.OperationalError) as excinfo:
with kb.write_txn(wrapper):
kb._append_event(wrapper, "t_bogus", "promoted", None)

msg = str(excinfo.value)
assert "disk I/O error" in msg, (
f"write_txn masked the original exception with rollback failure; "
f"got {msg!r} (expected to contain 'disk I/O error')"
)
assert "cannot rollback" not in msg, (
f"write_txn surfaced the rollback failure instead of the original "
f"OperationalError; got {msg!r}"
)