fix(kanban): close decomposer SQLite connections#32135
Conversation
Use contextlib.closing for kanban_decompose helpers so gateway ticks close SQLite handles deterministically. Add regression coverage for list_triage_ids().
hclsys
left a comment
There was a problem hiding this comment.
Verified — the fix is correct and safe, and I checked the write-path concern that contextlib.closing usually raises.
contextlib.closing(...).__exit__ calls only conn.close(), not sqlite3.Connection.__exit__, so it drops the implicit commit that with conn: would do. Two of the four converted blocks write — kb.specify_triage_task(conn, ...) (hermes_cli/kanban_decompose.py:373) and kb.decompose_triage_task(conn, ...) (:442). Normally swapping with conn → closing(conn) there would silently lose those writes.
It's safe here only because kb.connect() opens with isolation_level=None (hermes_cli/kanban_db.py:1137) — i.e. autocommit mode, where each execute() commits immediately and Connection.__exit__ has no open transaction to commit. So the implicit commit was already a no-op, and closing() loses nothing. The fd leak (handle never closed) is real and this fixes all four sites including the per-tick list_triage_ids hot path. Good.
One forward-looking note, not a blocker: this correctness now silently depends on connect() staying isolation_level=None. If anyone later switches kanban to explicit transactions, blocks :373/:442 would start losing writes under closing() with no test catching it (the new FakeConn test only asserts .close() fired). A one-line comment at the write-block sites — "safe under autocommit (connect() uses isolation_level=None); revisit if transactions are introduced" — would make that dependency explicit for the next editor.
LGTM.
|
我这边已经基于该问题做了一个最小修复 PR：#32135。 修复点： 另外，上游已经有一个非常接近的相关 PR：#29525。我的这个 PR 只聚焦在 decomposer hot path 的已验证路径，避免把尚未复现的其它 |
|
Closing as already fixed on main — landed via commit |
What does this PR do?
Fixes the embedded kanban decomposer path so its SQLite connections are closed deterministically instead of relying on
sqlite3.Connection.__exit__.kanban_decompose.list_triage_ids()runs on every gateway dispatcher tick. In the current implementation,with kb.connect() as conn:only commits/rolls back and leaves the SQLite handle open. Over time that accumulateskanban.db/kanban.db-walfile descriptors and eventually tripsToo many open files/unable to open database filein long-lived gateway processes.This patch switches the decomposer helpers to
contextlib.closing(kb.connect())so the connection is always closed on block exit.Related Issue
Fixes #29610
Type of Change
Changes Made
hermes_cli/kanban_decompose.pywith kb.connect()withcontextlib.closing(kb.connect())in the decomposer helper paths, includinglist_triage_ids().tests/hermes_cli/test_kanban_decompose.pylist_triage_ids()closes the underlying connection.How to Test
`uv run --extra dev python -m pytest tests/hermes_cli/test_kanban_decompose.py tests/hermes_cli/test_kanban_decompose_db.py tests/gateway/test_kanban_notifier.py -q -o 'addopts='
`uv run --extra dev python -m pytest tests/hermes_cli/test_kanban_decompose.py::test_list_triage_ids_closes_kanban_connection -q -o 'addopts='
list_triage_ids()in a live gateway and confirmkanban.dbfd count stays flat.Checklist
Code
fix(scope):,feat(scope):, etc.)Documentation & Housekeeping
Screenshots / Logs
Relevant verification:
Notes
Related upstream work already in flight:
kanban_decompose.pyhot path.