Skip to content

fix(kanban): fail closed and serialize sqlite writes - #31740

Closed
usmch1134-droid wants to merge 2 commits into
NousResearch:mainfrom
usmch1134-droid:fix/kanban-sqlite-hardening
Closed

fix(kanban): fail closed and serialize sqlite writes#31740
usmch1134-droid wants to merge 2 commits into
NousResearch:mainfrom
usmch1134-droid:fix/kanban-sqlite-hardening

Conversation

@usmch1134-droid

Copy link
Copy Markdown

Summary

  • fail closed on generic SQLite disk I/O error instead of treating it as WAL-incompatible fallback
  • serialize Kanban writes with a per-database interprocess file lock before BEGIN IMMEDIATE
  • disable Kanban boards by DB fingerprint after fatal storage errors so the gateway stops retrying known-bad DBs
  • add regression coverage for fatal error classification, write lock ordering, and multiprocess Kanban integrity

Why

On WSL under concurrent Kanban dispatcher/worker load, a board hit SQLite B-tree corruption after generic disk I/O error was handled like a safe WAL fallback. That let workers continue against degraded storage state. This makes IOERR/malformed/not-a-db fail closed and adds app-level write serialization around board mutations.

Test plan

  • PYTHONPATH=. /home/usmc1/.hermes/hermes-agent/venv/bin/python -m pytest tests/test_hermes_state_wal_fallback.py tests/hermes_cli/test_kanban_db.py tests/hermes_cli/test_kanban_multiprocess_integrity.py tests/gateway/test_kanban_sqlite_fatal_errors.py -o addopts= -q
    • Result after rebase onto upstream main: 192 passed, 1 warning in 12.78s
  • Earlier heavy temp-DB stress on Beast WSL: 8 processes x 300 iterations, PRAGMA integrity_check => ok, counts tasks=480 comments=960 events=1846 runs=406

Operational notes

  • Does not attempt to repair existing corrupted boards.
  • Operators should initialize a fresh board after deploying this patch.
  • For Beast WSL recovery, resume dispatch conservatively (--max 1) and run integrity checks before/after batches until confidence is rebuilt.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the focused SQLite-hardening investigation. The WAL-fallback half is already on current main, but this branch cannot be salvaged as a direct cherry-pick.

Problems

  • hermes_cli/kanban_db.py:1546 in the PR restores an unguarded ROLLBACK. Current main protects this at hermes_cli/kanban_db.py:2322-2329 so SQLite's auto-rollback does not hide the original EIO/corruption exception.
  • hermes_cli/kanban_db.py:1539 assumes every connection returns a file-backed PRAGMA database_list row. Current boundary tests use a fake connection whose execute() returns None (tests/hermes_cli/test_kanban_write_txn_busy_retry.py:17-36,62), so this would fail before BEGIN IMMEDIATE.
  • The reported WAL downgrade is already fixed by 5c49cd0e; main now re-raises EIO rather than falling back to DELETE (hermes_state.py:398-408). Main also has narrower dispatcher locking and write-boundary BUSY retry (hermes_cli/kanban_db.py:1416-1497,2296-2341).

Suggested changes

  • Rebase the design on current write_txn() without losing its retry, protected rollback, and cleanup behavior.
  • Reproduce a remaining current-main failure before adding global write serialization, and distinguish transient EIO from confirmed DB corruption.

Automated hermes-sweeper review.

Comment thread hermes_cli/kanban_db.py
lock_path = db_path.with_suffix(db_path.suffix + ".write.lock")
with _interprocess_file_lock(lock_path):
conn.execute("BEGIN IMMEDIATE")
try:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

write_txn() is currently exercised with fake boundary-test connections whose execute() returns None (tests/hermes_cli/test_kanban_write_txn_busy_retry.py:17-36,62), so this path lookup fails before BEGIN IMMEDIATE. Keep non-file-backed/test connections supported or explicitly branch before requiring PRAGMA database_list.

Comment thread hermes_cli/kanban_db.py
raise
else:
conn.execute("COMMIT")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please retain the protected rollback behavior now on main: SQLite may have already auto-rolled back after EIO or corruption, and an unguarded ROLLBACK then masks the original storage error with cannot rollback - no transaction is active.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jul 13, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Merged via #68654. Superseded by changes on main (WAL fallback landed in 5c49cd0; the guarded ROLLBACK on main is deliberate) and by #68654's narrow auto-repair. Thanks for the early attention to kanban write safety.

@teknium1 teknium1 closed this Jul 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants