Skip to content

fix(kanban): quarantine corrupt DB backups by fingerprint - #33529

Closed
hanzckernel wants to merge 2 commits into
NousResearch:mainfrom
hanzckernel:hz/kanban-corrupt-backup-quarantine
Closed

fix(kanban): quarantine corrupt DB backups by fingerprint#33529
hanzckernel wants to merge 2 commits into
NousResearch:mainfrom
hanzckernel:hz/kanban-corrupt-backup-quarantine

Conversation

@hanzckernel

@hanzckernel hanzckernel commented May 27, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Stops corrupt Kanban DB backup amplification and tightens gateway handling for quarantined corrupt boards.

When a board DB is corrupt today, repeated gateway ticks / health probes / restarts can keep opening the same malformed DB and keep creating timestamped *.corrupt.*.bak files. This PR makes corrupt backup preservation durable and idempotent for the same DB/WAL/SHM content fingerprint, while still preserving new bytes separately when the corrupt DB or its WAL/SHM sidecars change.

It also teaches the embedded gateway dispatcher and ready/review health probes to skip a board already paused for the same corrupt DB fingerprint, so probes do not immediately reopen the same bad board after dispatch paused it.

Related Issue

Refs #30445
Refs #31736

No new issue opened: the remaining problem is covered by those existing Kanban corruption / gateway pressure issues, and this PR is a focused follow-up fix.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • hermes_cli/kanban_db.py

    • Add a durable corrupt-quarantine marker next to the DB.
    • Fingerprint corrupt DB artifacts by DB/WAL/SHM exists + size + sha256 content, not mtime.
    • Reuse the first complete backup for unchanged corrupt content, even across retries/restarts or metadata-only touches.
    • Create a new deterministic backup when corrupt DB bytes or WAL/SHM sidecar bytes change.
    • Validate main and sidecar backup artifacts before reusing a marker.
    • Atomically repair missing/partial deterministic backups via temp file + fsync + os.replace.
    • Ignore missing/invalid marker backup paths rather than letting them suppress preservation.
    • Clear stale corrupt markers after a healthy DB open.
  • gateway/run.py

    • Pause corrupt board dispatch by DB/WAL/SHM stat fingerprint.
    • Skip same-fingerprint corrupt boards in dispatch, ready/review health checks, and auto-decompose scans.
    • Retry/re-pause when the DB/WAL/SHM stat fingerprint changes or the quarantine retry interval expires.
  • Tests

    • Add coverage for unchanged corrupt backup reuse, metadata-only touch reuse, changed-content new backup, changed sidecar new backup, partial main/sidecar backup repair, invalid marker backup paths, stale marker clearing, and dispatcher/health-probe skip/retry behavior.
    • Align two existing Kanban guard tests with current crash-grace / Windows waitpid guard behavior so the full CI slices stay green.

How to Test

Commands run locally on macOS 26.3.1:

python -m pytest -o addopts='' \
  tests/run_agent/test_tls_fd_recycle_corruption.py \
  tests/hermes_cli/test_kanban_db.py::test_init_db_refuses_corrupt_existing_file \
  tests/hermes_cli/test_kanban_db.py::test_connect_refuses_corrupt_existing_file \
  tests/hermes_cli/test_kanban_db.py::test_repeated_corrupt_existing_file_reuses_durable_backup_marker \
  tests/hermes_cli/test_kanban_db.py::test_metadata_only_touch_reuses_existing_corrupt_backup \
  tests/hermes_cli/test_kanban_db.py::test_changed_corrupt_existing_file_gets_new_backup \
  tests/hermes_cli/test_kanban_db.py::test_same_size_corrupt_content_change_gets_new_backup \
  tests/hermes_cli/test_kanban_db.py::test_changed_corrupt_sidecar_gets_new_backup \
  tests/hermes_cli/test_kanban_db.py::test_repeated_corrupt_file_repairs_partial_deterministic_backup \
  tests/hermes_cli/test_kanban_db.py::test_matching_marker_without_valid_backup_does_not_suppress_backup \
  tests/hermes_cli/test_kanban_db.py::test_matching_marker_repairs_partial_sidecar_backup \
  tests/hermes_cli/test_kanban_db.py::test_healthy_db_open_clears_stale_corrupt_marker \
  tests/hermes_cli/test_kanban_db.py::test_locked_healthy_db_does_not_classify_as_corrupt \
  tests/hermes_cli/test_kanban_core_functionality.py::test_gateway_dispatcher_disables_corrupt_board_without_traceback \
  tests/hermes_cli/test_kanban_core_functionality.py::test_gateway_dispatcher_retries_corrupt_board_after_quarantine \
  tests/hermes_cli/test_kanban_core_functionality.py::test_gateway_dispatcher_retries_corrupt_board_after_sidecar_change \
  -q
# 26 passed in 1.48s

python -m pytest -o addopts='' tests/hermes_cli/test_kanban_db.py tests/hermes_cli/test_kanban_core_functionality.py -q
# 370 passed, 1 skipped in 12.73s

$HOME/.hermes/hermes-agent/venv/bin/python -m ruff check gateway/run.py hermes_cli/kanban_db.py tests/hermes_cli/test_kanban_db.py tests/hermes_cli/test_kanban_core_functionality.py
# All checks passed!

scripts/run_tests.sh tests/hermes_cli/test_kanban_db.py tests/hermes_cli/test_kanban_core_functionality.py tests/tools/test_kanban_tools.py tests/tools/test_windows_native_support.py -- -q
# 509 tests passed, 0 failed

git diff --check
python -m py_compile hermes_cli/kanban_db.py gateway/run.py

Also ran two independent review passes after the final fixes; both returned no blockers.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 26.3.1

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

For New Skills

N/A

Screenshots / Logs

N/A

@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 comp/gateway Gateway runner, session dispatch, delivery labels May 27, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Competes with #33319 which also implements fingerprint-based dedup of corrupt kanban backups. Both are part of the kanban corruption cluster (#26479 canonical). Also overlaps #32094 (broader quarantine hardening). Please coordinate.

@teknium1

Copy link
Copy Markdown
Contributor

Salvaged in PR #33804 (merged as commit 6f9182cb3) — Co-authored-by: hanzckernel trailer preserved on the merge commit.

Your conceptual approach (content-fingerprint backup naming to make the retry idempotent) was the right call. I shipped a slimmed version focused purely on the disk-amplification bug:

  • Kept: content-addressed backup filename via hashlib.sha256(db)[:16]. Same bytes → same filename → if not path.exists(): copy skips the work on retries.
  • Dropped from this salvage (kept the bug fix, deferred the rest): persistent .corrupt-quarantine.json marker (the backup file IS the marker), atomic temp+fsync+os.replace copy helper (shutil.copy2 is fine for a known-broken DB path), WAL+SHM sidecar fingerprinting at the backup layer (main DB hash is what distinguishes incidents), gateway-side _pause_corrupt_board helper extraction (DRY-only refactor), gateway-side WAL+SHM fingerprint extension, and the health-probe / auto-decompose skip-corrupt-boards paths.

Net diff: +66/-21 across 2 files (was +670/-70 across 5).

Live verification before merge:

Metric Before After
Backup files after 10 retries on unchanged corrupt bytes 11 1
Disk usage 1144 KB 104 KB
Amplification 11x 1x
Mutated corrupt bytes → second backup n/a ✓ different fingerprint

Tests: 449/449 kanban tests pass. New regression test test_repeated_corrupt_open_reuses_single_backup asserts the invariant directly.

A few of the things you addressed in this PR overlap with concerns already handled by other recent landings:

  • #30445 and #31736 were closed earlier today as substantially fixed by the cluster-wide defenses (WAL re-init skip dc98314fb, fd-close ebe04c66c, torn-write pragmas 6416dd518, quarantine retry timer c94ad8981).
  • The gateway-side dispatch skip already exists via the disabled_corrupt_boards map with 5-min retry; the WAL+SHM fingerprint extension and health-probe skip paths are orthogonal improvements we can revisit if there's a recurring issue we can pin to them.

Thanks for the careful diagnostic work — the bug repro you described in the PR body was tight enough that I was able to reproduce on current main in ~5 minutes and verify the fix end-to-end. Authorship preserved via co-author trailer; AUTHOR_MAP entry was already in place from your earlier PR #20311.

#33804

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 comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants