Skip to content

fix(kanban): serialize DB initialization - #24410

Closed
psionic73 wants to merge 1 commit into
NousResearch:mainfrom
psionic73:fix/kanban-db-init-race-pr-20260512-145705
Closed

fix(kanban): serialize DB initialization#24410
psionic73 wants to merge 1 commit into
NousResearch:mainfrom
psionic73:fix/kanban-db-init-race-pr-20260512-145705

Conversation

@psionic73

Copy link
Copy Markdown
Contributor

Summary

  • Serialize Kanban DB schema initialization with a process-local reentrant lock.
  • Keep the existing per-path initialization cache, but guard both cache checks and forced re-init invalidation.
  • Add a threaded regression test that opens the same board database concurrently and verifies additive migration columns are present.

Why

During gateway startup or dispatcher activity, multiple same-process threads can call kanban_db.connect() for the same board at nearly the same time. Without synchronization, they can race through schema creation and additive migration checks with stale snapshots. The schema/migrations are idempotent, but the race can still surface as initialization-time failures.

Test plan

Passed:

  • scripts/run_tests.sh tests/hermes_cli/test_kanban_db_init.py -q — 1 passed
  • scripts/run_tests.sh tests/hermes_cli/test_kanban_db.py tests/hermes_cli/test_kanban_db_init.py tests/hermes_cli/test_kanban_boards.py tests/hermes_cli/test_kanban_specify_db.py -q — 143 passed

Additional broader run attempted:

  • scripts/run_tests.sh tests/hermes_cli/test_kanban_db.py tests/hermes_cli/test_kanban_db_init.py tests/hermes_cli/test_kanban_core_functionality.py tests/hermes_cli/test_kanban_boards.py tests/hermes_cli/test_kanban_specify_db.py -q
  • Result: 295 passed, 2 failed because this local hermetic test environment lacks fastapi, and two dashboard plugin tests import plugins.kanban.dashboard.plugin_api directly. The failures are environment dependency failures (ModuleNotFoundError: No module named 'fastapi'), not failures in this DB initialization change.

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/cron Cron scheduler and job management labels May 12, 2026
@psionic73

Copy link
Copy Markdown
Contributor Author

Final pre-merge review from Hermes Agent:

  • Scope reviewed: hermes_cli/kanban_db.py, tests/hermes_cli/test_kanban_db_init.py.
  • The change is narrow and addresses the same-process Kanban DB initialization race by serializing schema init/cache access while preserving the existing per-path cache behavior.
  • Regression coverage is present for concurrent same-board DB opens and additive migration columns.
  • Local verification on current PR head: scripts/run_tests.sh tests/hermes_cli/test_kanban_db_init.py -q -o 'addopts=' — 1 passed.
  • GitHub reports the PR as mergeable and no checks are reported for this fork branch.

Verdict: no blockers found; ready for maintainer merge.

@psionic73

Copy link
Copy Markdown
Contributor Author

@teknium1 sorry to ping directly — could you please review/merge these when you have a chance?

They are all currently mergeable and have fresh verification/review comments. I only have READ permission on the upstream repos, so I can’t merge them myself.

Hermes Agent:
- #24410 fix(kanban): serialize DB initialization
- #18310 fix(tui): preserve fallback provider chain
- #19331 feat: add source-bound cognee query tool

Paperclip adapter:
- NousResearch/hermes-paperclip-adapter#100 fix: enrich hermes local usage from state db

No GitHub checks are reported on these fork branches, but local targeted verification passed and is documented in the PR comments.

Thank you

@psionic73
psionic73 force-pushed the fix/kanban-db-init-race-pr-20260512-145705 branch from a5afce4 to 2138ec9 Compare May 13, 2026 16:56
@psionic73

Copy link
Copy Markdown
Contributor Author

Refreshed against current origin/main after upstream moved forward.

  • Rebased the existing Kanban DB initialization serialization fix onto current origin/main (a9b8254).
  • Force-with-lease updated the fork branch from a5afce4aca9c8fa53dbc9aee7240900afbeb88c7 to 2138ec9fc3884346bf2015972930c457904e686b.
  • Branch is now 1 commit ahead / 0 behind origin/main.
  • Fork branch is in sync with local rebased HEAD.
  • GitHub reports the PR as mergeable after recompute.
  • Checks: GitHub reports no checks for this fork branch.

Local verification:

  • scripts/run_tests.sh tests/hermes_cli/test_kanban_db_init.py -q -o 'addopts=' — 1 passed
  • python -m hermes_cli.main config check — config version 23 OK
  • git diff --check HEAD^..HEAD — OK

No intended functional scope changes beyond rebasing the existing Kanban PR over current main.

@psionic73
psionic73 force-pushed the fix/kanban-db-init-race-pr-20260512-145705 branch from 2138ec9 to bccf94a Compare May 14, 2026 10:15
@psionic73

Copy link
Copy Markdown
Contributor Author

Refreshed against current origin/main after upstream moved forward again.

  • Rebased the existing Kanban DB initialization serialization fix onto current origin/main (ddb8d8fa842283ef651a6e4514f8f561f736c72e).
  • New PR head: bccf94ad7d90007e87d31407da68999803d18c17.
  • Branch state after refresh: 1 commit ahead / 0 behind origin/main.
  • Fork/local parity verified after force-with-lease push: 0 ahead / 0 behind.
  • GitHub reports mergeable: MERGEABLE.
  • GitHub reports no checks.
  • Local verification: python -m pytest tests/hermes_cli/test_kanban_db_init.py -q → 1 passed.

No intentional functional scope change beyond refreshing the fix for current upstream; during refresh I kept WAL/schema initialization inside the same process-local init lock because the rebase exposed concurrent fresh-DB WAL activation as part of the same race class.

@wesleysimplicio

Copy link
Copy Markdown
Contributor

Reviewed the diff and the threaded regression test pattern. Reentrant lock + cache invalidation gating is the right shape — concurrent open against the same board DB now serializes cleanly. CI is green on the changed code (the build matrix failures earlier in the day are baseline / runner env). Single-file fix in hermes_cli/kanban_db.py with a tight test; matches @kshitijk4poor's salvage-batch criteria. /cc @kshitijk4poor for the next kanban wave.

@psionic73
psionic73 force-pushed the fix/kanban-db-init-race-pr-20260512-145705 branch from bccf94a to dcfb7cd Compare May 15, 2026 14:21
@psionic73

Copy link
Copy Markdown
Contributor Author

Quick refresh: rebased this PR onto current main again.

Current head: dcfb7cd058b106051017e4e50c46c259ed484f41
Base: d5416284f11ccbc735c8357f0ab35ce5f683ccc3
GitHub reports: MERGEABLE

Local verification:

  • tests/hermes_cli/test_kanban_db_init.py
  • tests/hermes_cli/test_kanban_db.py
  • tests/hermes_cli/test_kanban_core_functionality.py
  • tests/hermes_cli/test_kanban_specify_db.py

Result: 247 passed.

No CI checks are reported by GitHub for this PR at the moment. Given the focused scope and prior positive review, this should be ready for maintainer merge.

@psionic73
psionic73 force-pushed the fix/kanban-db-init-race-pr-20260512-145705 branch from dcfb7cd to b8bcbdb Compare May 19, 2026 00:03
@psionic73

Copy link
Copy Markdown
Contributor Author

Refreshed against current origin/main after upstream moved forward.

  • Rebased the Kanban DB initialization serialization fix onto current origin/main (378bca1d2ff8ffee9158016295d8b44e2cc81d59).
  • New PR head: b8bcbdbe24b1db583c3294941a84503667fa6a1a.
  • Branch is now 1 commit ahead / 0 behind origin/main.
  • Fork/local parity verified after force-with-lease push: 0 ahead / 0 behind.
  • GitHub reports mergeable: MERGEABLE (mergeStateStatus: UNSTABLE).
  • GitHub reports no checks for this fork branch.

Local verification:

  • scripts/run_tests.sh tests/hermes_cli/test_kanban_db_init.py tests/hermes_cli/test_kanban_db.py tests/hermes_cli/test_kanban_core_functionality.py tests/hermes_cli/test_kanban_specify_db.py -q -o 'addopts=' → 253 passed
  • git diff --check HEAD^..HEAD → OK

No intentional functional scope change beyond refreshing the existing Kanban DB initialization race fix onto current upstream.

@psionic73
psionic73 force-pushed the fix/kanban-db-init-race-pr-20260512-145705 branch from b8bcbdb to 9f9c226 Compare May 19, 2026 00:15
@psionic73

Copy link
Copy Markdown
Contributor Author

Rebased onto latest origin/main and refreshed the fork branch.

Verification performed locally:

  • python -m pytest tests/hermes_cli/test_kanban_db_init.py tests/cli/test_update_command.py -q -o 'addopts='
  • Result: 21 passed

Current head: 9f9c2265290b31fb2329bae453ab25d25455d9aa

@psionic73

Copy link
Copy Markdown
Contributor Author

Maintainer handoff: this PR has been refreshed against latest main, the fork branch is up to date, and GitHub currently reports MERGEABLE.

Local targeted verification passes:

  • python -m pytest tests/hermes_cli/test_kanban_db_init.py tests/cli/test_update_command.py -q -o 'addopts='
  • Result: 21 passed

GitHub currently reports no CI checks for this fork branch. From my side this is ready for review/merge.

@teknium1

Copy link
Copy Markdown
Contributor

Merged via PR #28383 (cherry-picked onto current main with your authorship preserved via rebase-merge — commit ca8126b). Thanks for the contribution!

@teknium1 teknium1 closed this May 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 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.

4 participants