Skip to content

fix(kanban): quarantine failures and capture coherent DB evidence - #66912

Open
chrismaddern wants to merge 2 commits into
NousResearch:mainfrom
chrismaddern:wt/t_745cdfe3
Open

fix(kanban): quarantine failures and capture coherent DB evidence#66912
chrismaddern wants to merge 2 commits into
NousResearch:mainfrom
chrismaddern:wt/t_745cdfe3

Conversation

@chrismaddern

Copy link
Copy Markdown

Summary

  • classify fatal SQLite failures and trip one board-wide incident circuit
  • add an exclusive maintenance lease with shared writer admission for quiescence
  • capture content-addressed, coherent main/WAL/SHM evidence bundles and atomically publish a reusable incident manifest
  • prevent capture while registered writers or write transactions are active

This supersedes #65899 with that Slice 1 work rebased onto current main, plus the dependent maintenance/evidence Slice 2.

Verification

  • scripts/run_tests.sh tests/hermes_cli/test_kanban_*.py tests/gateway/test_kanban_*.py -q: 762 passed
  • python -m ruff check hermes_cli/kanban_db.py hermes_cli/kanban_maintenance.py tests/hermes_cli/test_kanban_maintenance_e2e.py
  • python -m compileall -q ...
  • git diff --check

Behavioral evidence

  • subprocess-exclusive lease contention refuses the second holder
  • active writer registration and open write_txn both refuse evidence capture
  • restored WAL bundle passes PRAGMA integrity_check and retains all committed rows
  • repeated capture for the same incident reuses the completed manifest

@alt-glitch alt-glitch added type/bug Something isn't working comp/cron Cron scheduler and job management comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have needs-decision Awaiting maintainer decision before any implementation sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Jul 18, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.
This is an active supersession of CLOSED-unmerged #65899, not a duplicate. It is related to the Kanban SQLite quarantine work in #33929 and #41795; #66915 also carries a shared DB-health slice, so please choose a consolidation/split plan.

@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 extending the existing per-process dispatcher quarantine into a persisted board-health circuit. The current-main premise is real: gateway/kanban_watchers.py:943-1007 retains corrupt-board state only in memory and retries after 300 seconds.

Problems

  • hermes_cli/kanban_db.py:2041-2050 adds quarantine_db_for_error, but direct post-connect reads remain unwrapped: get_task() executes SQL at current-main hermes_cli/kanban_db.py:2706-2708, and list_tasks() at :2773. A fatal read error there will not create the persisted circuit.
  • hermes_cli/kanban_db.py:1968-1972 re-raises capture failures other than lease contention. Since _trip_db_health() writes health then calls this helper at :2024-2027, a copy/hash/fsync failure can mask the intended KanbanDbHealthError.

Suggested changes

  • Route post-connect SQL failures through one shared fatal-error boundary and add a read-path regression test.
  • Make evidence capture best-effort after health publication; test copy and manifest-write failures.
  • The subprocess lease tests explicitly skip Windows at tests/hermes_cli/test_kanban_maintenance_e2e.py:103-105; add Windows locking coverage for the new msvcrt path.

Automated hermes-sweeper review.

Comment thread hermes_cli/kanban_db.py
except Exception as exc:
from hermes_cli.kanban_maintenance import MaintenanceLeaseBusyError

if not isinstance(exc, MaintenanceLeaseBusyError):

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.

Only lease contention is converted to a deferred capture here. If shutil.copy2, hashing, or manifest fsync fails after _trip_db_health has published the health sidecar, this re-raise replaces the classified KanbanDbHealthError. Preserve the open circuit and report capture as incomplete for all evidence-capture I/O failures.

Comment thread hermes_cli/kanban_db.py
) -> Optional[KanbanDbHealthError]:
"""Persist a board circuit for a fatal SQLite error seen by a caller.

Operations that execute SQL after ``connect()`` use this boundary to feed

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 boundary is not applied to direct post-connect read helpers such as get_task and list_tasks, which still execute SQL without a SQLite-error wrapper. A fatal read error therefore bypasses the board-wide circuit; centralize execution/error conversion and cover a read-path failure.

return child, release


@pytest.mark.skipif(

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.

The new implementation has a separate msvcrt locking path, but every cross-process lease test is skipped on Windows. Add Windows coverage for exclusive-versus-shared admission before treating the lease as cross-platform evidence consistency.

@teknium1 teknium1 added 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 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 comp/gateway Gateway runner, session dispatch, delivery needs-decision Awaiting maintainer decision before any implementation 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-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants