Skip to content

fix(kanban): serialize all SQLite writers across processes - #67477

Open
o269 wants to merge 1 commit into
NousResearch:mainfrom
o269:fix/kanban-true-single-writer
Open

fix(kanban): serialize all SQLite writers across processes#67477
o269 wants to merge 1 commit into
NousResearch:mainfrom
o269:fix/kanban-true-single-writer

Conversation

@o269

@o269 o269 commented Jul 19, 2026

Copy link
Copy Markdown

Summary

Serializes every Hermes Kanban SQLite write transaction across processes with the board's existing .dispatch.lock, while preserving the dispatcher's non-blocking skip behavior.

This is a focused defense-in-depth fix for orphan gateways and worker stampedes: readers remain concurrent, but only one process may run a Kanban write transaction or close-time WAL checkpoint at once.

Fixes #53819.
Defense in depth for #35240.

Changes

  • Generalize the existing dispatch lock into a shared per-board file-lock primitive:
    • blocking/retrying mode for write transactions
    • non-blocking mode for dispatcher ticks
    • POSIX flock and Windows msvcrt.locking
    • same-thread reentrancy so dispatcher-owned writes do not deadlock
    • fail-closed timeout for unsafe concurrent writes
  • Wrap real SQLite write_txn() calls around the shared lock and use BEGIN IMMEDIATE with jittered busy retries at transaction boundaries.
  • Serialize connection initialization/migrations and explicit/GC connection close, because SQLite may checkpoint WAL during close.
  • Preserve existing dispatcher-lock semantics and mtime refresh behavior while making dispatcher and worker writers share the same lock.
  • Make the post-commit torn-extend probe WAL-checkpoint-aware: persistent mismatches still fail closed, while a live/incomplete legal checkpoint no longer produces a false corruption diagnosis.
  • Add multiprocessing regression coverage for dispatcher-vs-worker exclusion, transaction rollback/fail-closed behavior, same-thread nesting, and lock release after exceptions.
  • Update the existing WAL-fallback test fixture to compose with the new locking connection factory.

An applicable patch for the separate local bridge-state SQLite writer was also validated against the installed script (kanban_bridge_state.patch in the associated Kanban task workspace); it uses the same lock protocol and is intentionally not included as an unrelated host-local script in this repository PR.

Verification

Exact rebased tree (base ad0d21188):

  • HERMES_PYTHON=/home/odai/.hermes/hermes-agent/venv/bin/python scripts/run_tests.sh tests/hermes_cli/test_kanban*.py tests/tools/test_kanban*.py tests/gateway/test_kanban*.py tests/plugins/test_kanban*.py tests/agent/test_kanban_stop.py -q
    • 1,041 passed, 0 failed across 39 files
  • python -m ruff check hermes_cli/kanban_db.py tests/hermes_cli/test_kanban_db.py tests/hermes_cli/test_kanban_write_lock.py
    • passed
  • python -m py_compile ...
    • passed
  • git diff --check origin/main...HEAD
    • passed
  • uv build --quiet --out-dir /tmp/t_27b4be27-dist-final-rebased
    • sdist and wheel built successfully
  • Bridge patch dry-run/apply comparison + py_compile + Ruff
    • passed; patched output byte-matched the validated bridge script

Concurrent load gate

A mandatory 12-minute monitor ran against a fresh throwaway copy of the fleet board (the live board was read-only source material). It mixed:

  • 2 direct core API writers
  • 2 real hermes kanban CLI writer processes
  • 1 raw bridge-state SQLite writer using the companion lock patch
  • dispatcher ticks
  • task recycling
  • continuous SQLite quick/integrity probes

Result:

  • 721.1 seconds
  • 80,165 worker operations
  • 705 quick checks
  • 23 full integrity checks during load
  • all 7 worker exit codes: 0
  • worker errors: []
  • final PRAGMA quick_check: ok
  • final PRAGMA integrity_check: ok
  • final PRAGMA foreign_key_check: 0 rows

Risk / compatibility

  • Reads remain concurrent; only write transactions/checkpoints serialize.
  • Dispatcher ticks remain non-blocking and skip when another writer owns the lock.
  • Internal fake/wrapped test connections without an on-disk main path retain the pre-existing transaction behavior.
  • Lock timeout is bounded by HERMES_KANBAN_BUSY_TIMEOUT_MS and fails closed instead of silently falling back to unsafe concurrent writes.

@alt-glitch alt-glitch added type/bug Something isn't working comp/cron Cron scheduler and job management platform/windows Native Windows-specific behavior or breakage P3 Low — cosmetic, nice to have needs-decision Awaiting maintainer decision before any implementation sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows labels Jul 19, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to the open Kanban SQLite-corruption work (#31740, #45525, #60653), but this patch serializes all board writes and close-time checkpoints. Please choose whether to consolidate these overlapping approaches.

@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 single-writer hardening. Current main still has an unguarded normal write_txn() path at hermes_cli/kanban_db.py:2307-2344, while its existing .dispatch.lock only surrounds dispatch_once() at hermes_cli/kanban_db.py:7453-7502. The change therefore targets a current gap. Remote main is only one JS-only commit beyond this PR's base, so the Kanban changes should salvage cleanly.

Problems

  • hermes_cli/kanban_db.py:1515-1576 adds a distinct Windows msvcrt acquisition/release path, but tests/hermes_cli/test_kanban_write_lock.py:1-175 contains no Windows-path coverage. The multiprocessing checks validate the host fcntl path, not the added Windows behavior.

Suggested changes

  • Add a Windows-path regression for lock contention, bounded failure, and unlock-on-exception before relying on the cross-platform guarantee.

Automated hermes-sweeper review.

Comment thread hermes_cli/kanban_db.py
timeout_seconds = max(0.0, float(timeout_seconds))
deadline = time.monotonic() + timeout_seconds

if _IS_WINDOWS:

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.

This starts a separate Windows msvcrt implementation, but the added regression file has no _IS_WINDOWS/msvcrt coverage. Please add a Windows-path test for contention, bounded failure, and release after an exception.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cron Cron scheduler and job management needs-decision Awaiting maintainer decision before any implementation P3 Low — cosmetic, nice to have platform/windows Native Windows-specific behavior or breakage sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform 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.

bug: kanban DB corruption under high concurrent-worker load -- workers need per-write serialization

3 participants