fix: scan all kanban boards for active workers - #33923
the24thLetter wants to merge 29 commits into
Conversation
alt-glitch
added
P3
teknium1
left a comment
teknium1
left a comment
There was a problem hiding this comment.
Thanks for addressing the cross-board visibility gap: current main's endpoint reads only the current board (plugins/kanban/dashboard/plugin_api.py:1365-1366), while the embedded dispatcher enumerates all boards (gateway/kanban_watchers.py:1063-1078).
Problems
plugins/kanban/dashboard/plugin_api.py:1335now requiresr.id = t.current_run_id, but the unchanged worker-visibility test creates an open run without setting that field and expects one worker (tests/plugins/test_kanban_worker_runs.py:94-109). This regresses the existing contract.- The all-board loop at
plugins/kanban/dashboard/plugin_api.py:1508-1518does not deduplicate physical DB paths.HERMES_KANBAN_DBcan pin every slug to one DB (hermes_cli/kanban_db.py:521-532); the gateway explicitly deduplicates that case (gateway/kanban_watchers.py:209-232).
Suggested changes
- Preserve the existing open-run predicate, or update the contract and all affected tests coherently around
current_run_id. - Deduplicate resolved board DB paths before collecting snapshots.
- Separate the focused worker fix from the unrelated ledger, review-gate, script, and generated-asset work.
Automated hermes-sweeper review.
| AND r.worker_pid IS NOT NULL | ||
| AND t.status = 'running' | ||
| FROM tasks t | ||
| JOIN task_runs r ON r.id = t.current_run_id |
There was a problem hiding this comment.
This excludes open runs unless tasks.current_run_id was set. The unchanged test_workers_active_with_running_task creates an open run without setting that field and expects it to be listed (tests/plugins/test_kanban_worker_runs.py:94-109), so the suite and existing endpoint contract regress here.
| workers: list[dict[str, Any]] = [] | ||
| diagnostics: list[dict[str, Any]] = [] | ||
| board_summaries: list[dict[str, Any]] = [] | ||
| for board_slug in board_slugs: |
There was a problem hiding this comment.
Please deduplicate by resolved kanban_db.kanban_db_path(board_slug) before scanning. HERMES_KANBAN_DB can make every slug point to one DB (hermes_cli/kanban_db.py:521-532), which would duplicate every worker; gateway/kanban_watchers.py:209-232 already handles this case.
teknium1
added
sweeper:risk-session-state
GottZ
left a comment
GottZ
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Summary
Four PRs are in this cluster, but only #33923 directly targets the reported cross-board active-worker visibility gap; #33928 and #33929 combine a similar fleet-wide worker scan with broader SQLite quarantine and dashboard changes, while #33926 primarily implements the SQLite isolation work. None is merge-ready as-is: the focused worker implementations change the active-run contract, and the broader variants carry unrelated high-risk changes.
Related pull requests
- #33923
related— (+3550/-201) — keep open, not merge-ready: the diff makes an omitted board scan all boards, adds board/PID-source diagnostics, and falls back to tasks.worker_pid, directly addressing the visibility cause. The contributor keep_open review identifies two blockers that must be fixed first: preserve the existing open-run contract instead of requiring current_run_id, and deduplicate resolved physical DB paths when multiple slugs map to HERMES_KANBAN_DB. - #33926 [closed]
related— (+4209/-320) — superseded and not a direct consolidation target: this closed PR mainly adds classified per-board SQLite failure isolation and quarantine, while its diff does not contain the fleet-wide active-worker endpoint fix. It remains relevant as the earlier implementation lineage explicitly superseded by #33929. - #33928 [closed]
related— (+4537/-368) — superseded duplicate: this closed intermediate attempt combines fleet-wide worker/claim reporting with SQLite diagnostics and quarantine, but it is effectively the same broad change later published as #33929 and was based on the wrong integration branch. It remains relevant as the direct predecessor to #33929. - #33929
related— (+4537/-368) — do not merge as-is: its fleet-wide active-worker scan addresses the visibility cause and its disk-I/O/WAL diagnostics are distinct, but the contributor keep_open review is blocking because the extracted dispatcher drops default_assignee and max_in_progress_per_profile and modifies gateway/run.py after the watcher moved to gateway/kanban_watchers.py. Despite that keep_open review, consolidation should not continue on this exact PR: the diff is identical in size and substance to closed #33928, includes extensive unrelated fork-local changes, and requires a narrow reimplementation on current main rather than reopening or merging over the review.
Duplicates
#33928 and #33929 are essentially the same broad implementation; #33926 is an earlier SQLite-isolation predecessor superseded by #33929. Their worker-visibility portions overlap #33923, but #33923 is the more direct basis for the reported cause.
Suggested consolidation
Merge #33923 only after narrowing it to the active-worker endpoint and tests, preserving the existing open-run predicate, deduplicating resolved board DB paths, and addressing the contributor keep_open review. Close #33926 and #33928 as superseded; close #33929 after salvaging its distinct disk-I/O/WAL diagnostic enhancement into a separate current-main PR under gateway/kanban_watchers.py with default_assignee and max_in_progress_per_profile forwarding preserved and tested.
Cross-PR triage: Reviewed 4 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 893 kB of PR diffs, 6 kB of issue/PR text, 3 kB of discussion (6 comments), 3 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.
4 participants
Summary
Test Plan
Note: Running the full file without unsetting HERMES_KANBAN_REQUIRE_REVIEW_BEFORE_DONE in this Kanban worker environment fails existing dashboard tests that assume direct done completion instead of the Review gate.