Skip to content

fix(kanban): guard write_txn ROLLBACK against SQLite auto-abort#29839

Closed
crayfish-ai wants to merge 1 commit into
NousResearch:mainfrom
crayfish-ai:pr/kanban-write-txn-rollback-guard
Closed

fix(kanban): guard write_txn ROLLBACK against SQLite auto-abort#29839
crayfish-ai wants to merge 1 commit into
NousResearch:mainfrom
crayfish-ai:pr/kanban-write-txn-rollback-guard

Conversation

@crayfish-ai

Copy link
Copy Markdown
Contributor

Problem

When write_txn() catches an exception from its body and tries to ROLLBACK, SQLite may have already auto-aborted the transaction. The bare conn.execute("ROLLBACK") then raises OperationalError("cannot rollback - no transaction is active"), shadowing the original exception and making debugging extremely confusing.

This is a real failure mode — any code path inside write_txn that triggers an SQLite auto-abort (e.g. an IntegrityError, a constraint failure, or a nested ROLLBACK via another codepath) will hit this.

Fix

Guard the ROLLBACK with a try/except that swallows only the specific "cannot rollback" / "no transaction is active" errors. All other OperationalError variants are still re-raised. The original exception is preserved and propagated normally.

Test

Added test_write_txn_ignores_no_active_transaction_rollback_error — uses a mock connection that raises OperationalError("cannot rollback - no transaction is active") on every ROLLBACK call, then verifies that the original RuntimeError("boom") is still raised and that the ROLLBACK was attempted.

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/tools Tool registry, model_tools, toolsets labels May 21, 2026
@steveonjava

Copy link
Copy Markdown
Contributor

I also hit this issue and had the same fix. Great work, @crayfish-ai! 🎉

@crayfish-ai

Copy link
Copy Markdown
Contributor Author

Thanks for confirming! Glad the fix helped you too. If you have a moment, a review/approval would help get this merged faster. 🙏

When write_txn's body raises an exception, SQLite may have already
auto-aborted the transaction before we reach the explicit ROLLBACK.
The bare conn.execute('ROLLBACK') then raises OperationalError
('cannot rollback - no transaction is active'), which shadows the
original exception and makes debugging impossible.

Guard the ROLLBACK with a try/except that swallows only the
'cannot rollback' / 'no transaction is active' errors, preserving
the original exception.
@crayfish-ai crayfish-ai force-pushed the pr/kanban-write-txn-rollback-guard branch from eff69a8 to 25a96b3 Compare May 23, 2026 13:56
@steveonjava

Copy link
Copy Markdown
Contributor

I am not a maintainer, so I can't approve this. But it will probably go faster if you do the contributor attribution check. It looks like you are supposed to add your GitHub username to AUTHOR_MAP in the submission since this is your first PR.

⚠️ New contributor email(s) not in AUTHOR_MAP:

crayfish-ai@users.noreply.github.com (crayfish-ai)

Please add mappings to scripts/release.py AUTHOR_MAP:
"crayfish-ai@users.noreply.github.com": "",

To find the GitHub username for an email:
gh api 'search/users?q=EMAIL+in:email' --jq '.items[0].login'

@crayfish-ai

Copy link
Copy Markdown
Contributor Author

Thanks for the pointer on AUTHOR_MAP! I've added the email mapping now. Really appreciate you taking the time to explain — it's my first contribution to this repo and the CI setup isn't obvious. 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tools Tool registry, model_tools, toolsets P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants