fix(kanban): serialize shared board writes

[cbicudo]

What does this PR do?

Related Issue

Fixes #

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

How to Test

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:

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

• This skill is broadly useful to most users (if bundled) — see Contributing Guide
• SKILL.md follows the standard format (frontmatter, trigger conditions, steps, pitfalls)
• No external dependencies that aren't already available (prefer stdlib, curl, existing Hermes tools)
• I've tested the skill end-to-end: hermes --toolsets skills -q "Use the X skill to do Y"

Screenshots / Logs

[alt-glitch]

This was generated by AI during triage.

Duplicate of #67477. Both serialize Kanban SQLite writes using the shared board dispatch lock; #67477 is earlier and covers additional writer and checkpoint paths.

[teknium1]

Thanks for addressing a real concurrency gap: current write_txn() does not share the dispatch lock (hermes_cli/kanban_db.py:2751-2777), while dispatcher ticks do (hermes_cli/kanban_db.py:8105-8124).

Problems

• The new _common_write_lock() in de7ae8e7946508ea684cbea0ec81d90cca88d6be assumes conn.execute(...).fetchone()[2]. Existing _FakeConn.execute() returns None in tests/hermes_cli/test_kanban_write_txn_busy_retry.py:28-36, and that file calls write_txn() at lines 61-82. The patch would fail those existing tests before the transaction boundary.
• The new multiprocessing helper imports fcntl unconditionally, so its regression test is not native-Windows runnable. The current lock implementation explicitly supports Windows at hermes_cli/kanban_db.py:1516-1527.
• As noted in the member's duplicate comment, fix(kanban): serialize all SQLite writers across processes #67477 is the earlier broader approach. This patch does not cover the direct connection close in hermes_cli/kanban_db.py:2232-2239 or the additional writer/checkpoint paths that PR addresses.

Suggested changes

• Consolidate with fix(kanban): serialize all SQLite writers across processes #67477's shared, bounded lock primitive; retain a safe fallback for fake/wrapped connections and add platform-appropriate lock-holder coverage.

Automated hermes-sweeper review.

[GottZ]

This was generated by AI during triage.

Summary

Ten PRs address or reference the reported SQLite-corruption complex. The diffs split into broad per-write serialization proposals (#31965, #32461, #35770, #35787, #37292, #37344, #37350, #67477, and #73328) and the merged root-cause fix #71724, which removes raw file-descriptor reads that cancelled live POSIX SQLite locks; later contributor evidence also confounds the original concurrent-WAL-writer premise and identifies #71724 as the better fit for observed real damage.

Related pull requests

• fix(kanban): flock WAL init and write_txn against concurrent processes #31965 [closed] partial — (+553/-143) — n/a: Adds per-write and WAL-initialization flocking, but it is closed because the contributor review says fix(kanban): batch-salvage 7 SQLite corruption hardening fixes from #32857 #33482 and fix(kanban): salvage cross-process init flock + busy_timeout (from #32759) #33696 superseded its initialization work and rejected taxing every transaction for the then-identified cause.
• fix(kanban): add inter-process file lock to prevent DB corruption under concurrent writes #32461 [closed] partial — (+29/-0) — n/a: Adds an fcntl lock around every write_txn(), but the visible diff imports fcntl unconditionally and retains lock handles by id(conn) without lifecycle cleanup; it is closed as a duplicate of fix(kanban): harden SQLite connection concurrency #32759, merged via fix(kanban): salvage cross-process init flock + busy_timeout (from #32759) #33696.
• fix(kanban): add inter-process file lock on write_txn() to prevent DB corruption #35770 [closed] partial — (+46/-3) — n/a: Extends per-write locking to schema initialization, but the contributor review identifies an unconditional Windows-breaking fcntl import and an invalid file-object argument to flock; this closed precursor was superseded by the fuller fix(kanban): add inter-process flock to prevent SQLite WAL corruption #35787 iteration.
• fix(kanban): add inter-process flock to prevent SQLite WAL corruption #35787 [closed] partial — (+85/-8) — n/a: Serializes schema initialization and write_txn() with a per-DB flock and also includes unrelated DingTalk changes; although a non-contributor review approved it, the contributor discussion classifies it as overlapping fix(kanban): add inter-process file lock on write_txn() to prevent DB corruption #35770, and it was closed before a cleaner replacement.
• fix(kanban,send_message): 看板进程锁 + 钉钉 Markdown webhook / kanban flock + DingTalk markdown #37292 [closed] partial — (+85/-8) — n/a: Repeats fix(kanban): add inter-process flock to prevent SQLite WAL corruption #35787's Kanban locking and DingTalk Markdown changes in one diff; it is closed after the author was asked to split those unrelated concerns.
• fix(kanban): 看板跨进程写锁防止 SQLite WAL 损坏 / add inter-process flock #37344 [closed] partial — (+90/-3) — n/a: Refines the same flock design with optional fcntl, path-keyed handles, reverse connection lookup, and cleanup added after a contributor flagged FD leakage; it remains relevant as a closed precursor whose author reported the work merged upstream.
• fix(kanban): serialize cross-process SQLite writes #37350 [closed] fixes — (+425/-82) — n/a: Implements a bounded cross-platform sidecar lock around initialization and every write, adds extensive lock tests, and gates the file-length invariant for WAL; it is closed in favor of narrower fix(kanban): avoid WAL false corruption under worker bursts #45525 because its author concluded full serialization sacrificed normal WAL parallelism while the observed path was a false-positive detector.
• fix(kanban): serialize all SQLite writers across processes #67477 best fix — (+507/-146) — n/a; recorded best fix for bug: kanban DB corruption under high concurrent-worker load -- workers need per-write serialization #53819: Broadly shares the board dispatch lock across initialization, normal transactions, dispatcher activity, and close-time checkpoints with bounded fail-closed acquisition. The contributor keep-open review rates salvageability high and requires Windows contention, timeout, and exception-unlock coverage before relying on its cross-platform behavior.
• fix(state): stop cancelling our own POSIX locks on live SQLite databases #71724 [merged] best fix — (+1054/-79) — n/a; recorded best fix for bug: kanban DB corruption under high concurrent-worker load -- workers need per-write serialization #53819 and state.db schema corruption on macOS: verify against #71631 + #71724 (original _reconcile_columns DDL premise did not reproduce) #71480: This merged reference implementation fixes the measured lock-cancellation cause by replacing unsafe raw database reads, tracking live connections, and making the Kanban invariant WAL-aware; independent contributor review reports the prior blockers resolved and 260 targeted tests passing on the reviewed head.
• fix(kanban): serialize shared board writes #73328 partial — (+143/-19) — n/a: Narrowly makes write_txn() share the dispatch lock, but the visible diff assumes a real PRAGMA database_list row, uses a POSIX-only test helper, and omits the close/checkpoint paths covered by fix(kanban): serialize all SQLite writers across processes #67477. Despite the maintainer-bot keep-open verdict on fix(kanban): serialize shared board writes #73328, the diff is a strict, less-complete overlap with the earlier recorded best fix fix(kanban): serialize all SQLite writers across processes #67477 and the bot itself directs consolidation there.

Duplicates

#32461, #35770, #35787, #37292, #37344, #37350, #67477, and #73328 overlap on cross-process Kanban write serialization; #35787 and #37292 are effectively the same mixed Kanban/DingTalk change, while #73328 is the narrower duplicate of #67477. #31965 additionally overlaps the already-closed WAL-initialization chain #33482/#33696; #71724 is complementary root-cause work rather than merely another serialization duplicate.

Suggested consolidation

Keep #67477 open with a salvage path: rebase it onto the merged #71724 baseline, retain its shared bounded lock coverage for normal writes and close-time checkpoints, and add the contributor-requested Windows contention, bounded-failure, and unlock-on-exception tests. Close #73328 as a duplicate of #67477 despite its keep-open review because its diff covers only write_txn(), breaks existing fake-connection expectations, lacks native-Windows lock-holder coverage, and omits #67477's additional writer/checkpoint paths; the remaining listed alternatives are already closed, while merged #71724 should remain the root-cause reference for both issues.

Complex graph

flowchart TD
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I53819(["issue #53819 (open)"])
    subgraph Dup31965 ["PRs duplicating each other"]
        P31965["PR #31965 (closed)"]
        P32461["PR #32461 (closed)"]
        P35770["PR #35770 (closed)"]
        P35787["PR #35787 (closed)"]
        P37292["PR #37292 (closed)"]
        P37344["PR #37344 (closed)"]
        P37350["PR #37350 (closed)"]
        P67477["PR #67477 (open)"]
        P73328["PR #73328 (open)"]
    end
    P73328 -.->|partial| I53819
    class I53819 open
    class P31965 closed
    class P32461 closed
    class P35770 closed
    class P35787 closed
    class P37292 closed
    class P37344 closed
    class P37350 closed
    class P67477 open
    class P73328 open
    class P67477 best
    class P73328 target
    click I53819 "https://github.com/NousResearch/hermes-agent/issues/53819"
    click P31965 "https://github.com/NousResearch/hermes-agent/pull/31965"
    click P32461 "https://github.com/NousResearch/hermes-agent/pull/32461"
    click P35770 "https://github.com/NousResearch/hermes-agent/pull/35770"
    click P35787 "https://github.com/NousResearch/hermes-agent/pull/35787"
    click P37292 "https://github.com/NousResearch/hermes-agent/pull/37292"
    click P37344 "https://github.com/NousResearch/hermes-agent/pull/37344"
    click P37350 "https://github.com/NousResearch/hermes-agent/pull/37350"
    click P67477 "https://github.com/NousResearch/hermes-agent/pull/67477"
    click P73328 "https://github.com/NousResearch/hermes-agent/pull/73328"

Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 10 pull requests and 2 issues in this complex. Each diff was read against this issue; Assessment working set: 155 kB of PR diffs, 33 kB of issue/PR text, 43 kB of discussion (34 comments), 16 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.