Skip to content

Fix kanban dispatcher SQLite connection churn - #31768

Closed
sntrblck-sudo wants to merge 1 commit into
NousResearch:mainfrom
sntrblck-sudo:fix/kanban-dispatcher-reuse-conn
Closed

Fix kanban dispatcher SQLite connection churn#31768
sntrblck-sudo wants to merge 1 commit into
NousResearch:mainfrom
sntrblck-sudo:fix/kanban-dispatcher-reuse-conn

Conversation

@sntrblck-sudo

Copy link
Copy Markdown

Reuses the board connection for the readiness probe so the embedded gateway dispatcher does not open a second SQLite handle per board tick.

@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 isolating the duplicate readiness-probe connection. The performance premise still holds on current main: gateway/kanban_watchers.py:1009 opens the dispatch connection, while gateway/kanban_watchers.py:1100 opens a separate connection for health telemetry after each tick.

Problems

  • The submitted hunk targets the pre-extraction implementation in gateway/run.py. GatewayRunner now inherits GatewayKanbanWatchersMixin at gateway/run.py:2775, and the active watcher is in gateway/kanban_watchers.py; applying this diff would not update the live path without a focused port.
  • gh pr diff 31768 shows substantial unrelated gateway reversions outside the dispatcher hunk, so the current diff is not safely scoped to the claimed fix.

Suggested changes

  • Port only the tuple/readiness flow into gateway/kanban_watchers.py:975-1254, reusing the conn opened at line 1009 for has_spawnable_ready and has_spawnable_review.
  • Move the regression coverage to that active path and assert one connection per board tick.

Automated hermes-sweeper review.

Comment thread gateway/run.py
board=slug,
max_spawn=max_spawn,
max_in_progress=max_in_progress,
failure_limit=failure_limit,

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 is the right narrow behavior, but current main no longer executes this watcher from gateway/run.py: GatewayRunner inherits GatewayKanbanWatchersMixin (gateway/run.py:2775), whose live connection/probe paths are gateway/kanban_watchers.py:1009 and :1100. Port this hunk to that mixin rather than applying it at the removed location.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jul 13, 2026
@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Thanks @sntrblck-sudo — the kanban dispatcher connection-reuse idea is sound (one connection per tick instead of open/close churn, correctly scoped inside the to_thread try/finally). Unfortunately this branch was cut from a stale checkout: alongside the ~40-line kanban fix, the commit reverts several unrelated features that landed on main earlier (gateway transient-network crash handling from #31066/#31110, media delivery-path filtering at 3 send sites, status-bubble editing from #30045, env-var config expansion, fallback-chain key resolution, and more), so it can't merge as-is. There's also one behavioral note on the fix itself: folding the readiness probe into the same tick/connection after dispatch_once masks the 'queue non-empty but dispatch broken' condition the stuck-tick watchdog exists to catch — the old independent probe still fired in that state. Closing this one; the connection-reuse approach is worth re-extracting onto current main with the watchdog probe kept independent, and we'd be glad to review that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) 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-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/perf Performance improvement or optimization

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants