Skip to content

Fix Kanban DB write safety and JSON error handling - #33491

Open
ha001985 wants to merge 1 commit into
NousResearch:mainfrom
ha001985:fix/kanban-db-stability
Open

Fix Kanban DB write safety and JSON error handling#33491
ha001985 wants to merge 1 commit into
NousResearch:mainfrom
ha001985:fix/kanban-db-stability

Conversation

@ha001985

Copy link
Copy Markdown

Summary

Adds busy_timeout lock-contention mitigation, write_txn depth tracking keyed by connection/thread, per-board write serialization, corrupt/init JSON handling so Desktop does not get empty stdout, and preserves the list --json array contract.

Changes

  • DB write safety: busy_timeout raised to 30s; write_txn depth tracking per connection/thread to detect and reject nested writes
  • Per-board serialization: boards now serialize all writes through a dedicated RLock path
  • Corrupt/init JSON guard: list --json catches corrupt/invalid JSON and falls back gracefully instead of emitting empty stdout
  • Contract preservation: list --json always returns a JSON array (never empty, never malformed)

Tests

  • Test proof: 350 passed, 1 skipped
  • Stress proof: 8 threads × 20 writes, integrity_check=ok; 5 writers + 5 readers, integrity_check=ok
  • Live validation: 20 tasks created, 5 links, 5 archived, block/complete/unblock operations all succeeded, JSON parse confirmed valid, dry-run dispatch clean

Validation

Live validation was run on a fresh test board kanban-postfix-validation with the following results:

  • 20 tasks created, 5 dependency links established, 5 tasks archived
  • Block/complete/unblock operations all succeeded
  • hermes kanban list --json returned valid JSON with correct status breakdown (ready=10, todo=2, done=2, blocked=1)
  • PRAGMA integrity_check returned ok at every phase
  • Dry-run dispatch executed without error

Breaking changes

None. The Kanban board is backwards-compatible with existing data. Gateway auto-dispatch remains disabled by default (HERMES_KANBAN_DISPATCH_IN_GATEWAY=0) as the correct production default.

@ha001985
ha001985 force-pushed the fix/kanban-db-stability branch from b1c4f66 to 96aa509 Compare May 27, 2026 22:00
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard labels May 27, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Competes with #33325 and #32759 — all three add busy_timeout and write-safety hardening to kanban SQLite. Root issue: #32532. Consider consolidating.

@ha001985

Copy link
Copy Markdown
Author

Competes with #33325 and #32759 — all three add busy_timeout and write-safety hardening to kanban SQLite. Root issue: #32532. Consider consolidating.

Thanks for the context. I’m happy to consolidate with #33325 / #32759 and align this PR with the preferred direction.

This PR overlaps on busy_timeout, but it also includes a few additional pieces that may be useful for the final consolidated fix:

  • write_txn depth tracking keyed by connection/thread
  • per-board write serialization behavior
  • corrupt/init JSON handling so Desktop does not receive empty stdout
  • preservation of the list --json array contract for the Kanban UI
  • cmd_kanban exit-code propagation
  • focused regression tests and concurrency stress tests

Validation performed locally:

  • tests/hermes_cli/test_kanban_db.py → 175 passed
  • tests/hermes_cli/test_kanban_core_functionality.py → 175 passed, 1 skipped
  • stress: 8 threads × 20 writes → integrity_check=ok
  • stress: 5 writers + 5 readers → integrity_check=ok
  • live validation board: 20 tasks, 5 links, 5 archived, block/complete/unblock operations, list --json parse check, integrity_check=ok at every phase

Please let me know whether you’d prefer:

  1. I rebase this PR on top of one of the existing PRs,
  2. I split out only the non-overlapping parts,
  3. or I close this PR and move the useful pieces into the preferred consolidation branch.

@ha001985
ha001985 force-pushed the fix/kanban-db-stability branch from 96aa509 to 97cc148 Compare May 28, 2026 05:57
…t_lock and _sqlite_connect for busy_timeout
@ha001985
ha001985 force-pushed the fix/kanban-db-stability branch from 97cc148 to 64db74d Compare May 28, 2026 06:56
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the detailed validation and for offering to consolidate with the related write-safety PRs.

Problems

  • The proposed dispatcher pidfile is not an atomic lock: two processes can both pass the existence check, overwrite the file, and dispatch concurrently. Current main instead has a board-scoped non-blocking OS file lock in hermes_cli/kanban_db.py:1415-1497, used by dispatch at hermes_cli/kanban_db.py:7222.
  • The _cmd_init() error handling cannot handle initialization failures because kanban_command() calls kb.init_db() before handler dispatch (hermes_cli/kanban.py:924-936).
  • The fixed 30-second busy-timeout tests are stale: current main defaults to 120 seconds at hermes_cli/kanban_db.py:1287-1313 and already retries safe transaction boundaries in commit 204a67f0c85b721f55bd11a924c5cd814c09cbe6.

Suggested changes

  • Preserve the still-needed list --json corrupt-init array response in the pre-dispatch error path, with a focused regression test.
  • Drop the pidfile and timeout/RLock portions in favor of current main's lock and retry mechanisms.

Automated hermes-sweeper review.

@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-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 2026
@alt-glitch alt-glitch added comp/cron Cron scheduler and job management and removed comp/cli CLI entry point, hermes_cli/, setup wizard sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows labels Jul 13, 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 P3 Low — cosmetic, nice to have 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-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