Skip to content

fix: harden kanban sqlite write failures - #31952

Open
ryandidurlabs wants to merge 2 commits into
NousResearch:mainfrom
ryandidurlabs:cody/kanban-sqlite-hardening-t_15acb1eb
Open

fix: harden kanban sqlite write failures#31952
ryandidurlabs wants to merge 2 commits into
NousResearch:mainfrom
ryandidurlabs:cody/kanban-sqlite-hardening-t_15acb1eb

Conversation

@ryandidurlabs

Copy link
Copy Markdown

Summary

  • preserve the original SQLite write/dispatcher lease error when rollback also fails, instead of masking disk I/O failures with cannot rollback - no transaction is active
  • reject writes on stale connections when the main DB path is missing, zero-byte, or structurally truncated/malformed before BEGIN IMMEDIATE
  • treat zero-byte DBs with durable board-state markers and zero-task DBs with dependent Kanban rows as corrupt/inconsistent instead of silently reinitializing empty boards
  • escape zero-byte backup-marker glob patterns for unusual explicit DB filenames

Test plan

  • env -u HERMES_KANBAN_DB -u HERMES_KANBAN_BOARD -u HERMES_KANBAN_WORKSPACES_ROOT -u HERMES_KANBAN_TASK -u HERMES_KANBAN_WORKSPACE -u HERMES_KANBAN_RUN_ID -u HERMES_KANBAN_CLAIM_LOCK python -m pytest tests/hermes_cli/test_kanban_db.py -q (182 passed)
  • env -u HERMES_KANBAN_DB -u HERMES_KANBAN_BOARD -u HERMES_KANBAN_WORKSPACES_ROOT -u HERMES_KANBAN_TASK -u HERMES_KANBAN_WORKSPACE -u HERMES_KANBAN_RUN_ID -u HERMES_KANBAN_CLAIM_LOCK python -m pytest tests/hermes_cli/test_kanban_db.py tests/hermes_cli/test_kanban_diagnostics.py tests/hermes_cli/test_kanban_db_init.py tests/hermes_cli/test_kanban_boards.py tests/hermes_cli/test_kanban_core_functionality.py tests/hermes_cli/test_kanban_cli.py tests/tools/test_kanban_tools.py -q (588 passed, 1 existing discord audioop deprecation warning)
  • python -m ruff check .
  • git diff --check and diff static scan clean
  • independent Cody subagent review passed with no security concerns or blocking logic errors

Operational note

After review/merge, restart or roll out the Hermes gateway/dispatcher process so long-lived dispatcher instances pick up this code. Kanban child task t_8322370b tracks that operational rollout.

Add dispatcher lease coordination, corrupt DB detection, safer reconciliation, and Kanban regression coverage.\n\nTask: t_a0e3aea4
@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 25, 2026

@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 detailed failure coverage. The stale-connection preflight remains relevant: current main's write_txn() still proceeds directly to BEGIN IMMEDIATE at hermes_cli/kanban_db.py:2319, although rollback masking is already handled at lines 2322-2330.

Problems

  • tests/hermes_cli/test_kanban_core_functionality.py:3487-3496 adds exact default-value assertions (120, 3, 4). These are change-detector tests rather than behavioral contracts.
  • The PR's dispatcher lease at gateway/run.py:5361-5371 overlaps a later architecture: current main uses the board-scoped dispatch lock in hermes_cli/kanban_db.py:7222-7237, and watcher code now lives in gateway/kanban_watchers.py.

Suggested changes

  • Port the stale-write validation narrowly onto current write_txn() while retaining its current retry/rollback behavior.
  • Split the lease, reconciliation, gateway filtering, and default changes from the SQLite-write fix; replace the exact-default tests with invariants.

Automated hermes-sweeper review.

interval = kanban.get("dispatch_interval_seconds")
assert isinstance(interval, (int, float)) and interval >= 1, (
f"dispatch_interval_seconds must be a positive number, got {interval!r}"
assert interval == 120, (

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 exact default-value assertion is a change-detector test: an intentional future cadence adjustment fails CI without proving a broken behavior. Please test the configuration invariant or parser/dispatcher behavior instead.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-automation Sweeper risk: may affect CI, automerge, label sync, or maintainer automation sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-automation Sweeper risk: may affect CI, automerge, label sync, or maintainer automation sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages 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