fix(kanban): add dispatcher preflight guards - #28642
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the focused dispatcher hardening work. I verified the premise against current main: the ready-task spawn path still goes claim → resolve workspace → spawn with no selection-packet or duplicate-workspace preflight at hermes_cli/kanban_db.py:6282.
Problems
- hermes_cli/kanban_db.py:4600 adds
HERMES_KANBAN_STRICT_DISPATCH_PREFLIGHTfor non-secret behavior. AGENTS.md:107 says these settings belong inconfig.yaml, not new user-facingHERMES_*env vars. - hermes_cli/kanban_db.py:4643 checks whether
dirworkspaces already exist, but dispatch callsresolve_workspacefirst, and current main createsdirworkspaces at hermes_cli/kanban_db.py:4735. That branch is effectively dead fordirworkspaces. - The default dispatcher is gateway-embedded. gateway/kanban_watchers.py:1021 only logs when workers spawn and does not include
preflight_blocked, so the PR's deprecated CLI daemon logging does not cover the main operator path. - Current main also spawns review agents through the separate review path at hermes_cli/kanban_db.py:6368; the duplicate-workspace guard needs to cover that sibling path too.
Suggested changes
- Use
kanban.strict_dispatch_preflightonly, and updateDEFAULT_CONFIG/docs if this is a supported knob. - Make workspace validation run before path creation, or remove unreachable checks.
- Apply the guard/reporting to both ready and review dispatch paths, including gateway logs.
This is an automated hermes-sweeper review.
| These stricter checks are opt-in so lightweight boards are not forced into | ||
| a CAT-style task contract overnight. | ||
| """ | ||
| env = os.environ.get("HERMES_KANBAN_STRICT_DISPATCH_PREFLIGHT", "").strip().lower() |
There was a problem hiding this comment.
This introduces a new user-facing HERMES_* toggle for non-secret behavior. AGENTS.md says behavioral settings should live in config.yaml; please use kanban.strict_dispatch_preflight without the env surface.
| kind = task.workspace_kind or "scratch" | ||
| if kind in {"dir", "worktree"} and not Path(str(workspace)).is_absolute(): | ||
| return f"workspace path is not absolute: {workspace}" | ||
| if kind == "dir" and not workspace.exists(): |
There was a problem hiding this comment.
This dir missing check runs after resolve_workspace, which creates dir workspaces before preflight, so the branch is effectively unreachable for dir tasks.
| continue | ||
| # Persist the resolved workspace path so the worker can cd there. | ||
| set_workspace_path(conn, claimed.id, str(workspace)) | ||
| if spawn_fn is None: |
There was a problem hiding this comment.
Tying preflight to spawn_fn is None means custom spawn implementations bypass the dispatcher invariant. If this is a real pre-spawn guard, it should run before any spawn function.
| @@ -2149,13 +2153,14 @@ def _on_tick(res): | |||
| return | |||
There was a problem hiding this comment.
This updates the deprecated standalone daemon output, but the default dispatcher runs in the gateway; gateway/kanban_watchers.py also needs to surface preflight-blocked ticks.
GottZ
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Summary
Two PRs address unsafe Kanban worker dispatch: #28638 combines dispatcher preflight checks with broader worker-lifecycle and gateway changes, while #28642 focuses on selection-packet, workspace-conflict, optional contract/worktree checks, and preflight reporting.
Related pull requests
- #28638 [closed]
duplicate— (+868/-49) — closed reference: The diff adds the relevant pre-spawn guards but also changes stale-claim handling, timeout recovery, human-task dispatch, post-completion worker exit, and gateway shutdown notifications. It remains relevant as the broader implementation reference and was closed in favor of a clean non-stacked PR, without establishing that #28642 was mechanically extracted from it. - #28642
related— (+211/-3) — keep-open salvage: The focused diff directly blocks selection packets and duplicate running workspace writers before the normal worker spawn and exposes preflight failures. The contributor keep_open review identifies required corrections: use config.yaml instead of a new HERMES_* setting, remove or reposition the ineffective post-resolution dir-existence check, report failures through gateway/kanban_watchers.py, and cover the separate review-agent spawn path.
Duplicates
#28638 and #28642 substantially duplicate the dispatcher-preflight implementation for selection-packet rejection, duplicate-workspace prevention, strict contract/worktree validation, and preflight reporting; #28638 additionally contains several unrelated worker-lifecycle and gateway changes.
Suggested consolidation
Keep #28642 open with a salvage path consistent with its contributor keep_open review: retain the focused selection-packet and duplicate-workspace guards, move strict-mode configuration exclusively to config.yaml, correct or remove the dead dir-workspace check, surface preflight failures in gateway/kanban_watchers.py, and extend the protection to the review-agent spawn path. Keep #28638 closed as the broader, stacked predecessor/reference; its overlapping preflight portion is duplicated by #28642, while its unrelated changes should not be folded into this focused fix without separate review.
Complex graph
flowchart LR
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
subgraph Dup28638 ["PRs duplicating each other"]
P28638["PR #28638 (closed)"]
P28642["PR #28642 (open)"]
end
class P28638 closed
class P28642 open
class P28642 target
click P28638 "https://github.com/NousResearch/hermes-agent/pull/28638"
click P28642 "https://github.com/NousResearch/hermes-agent/pull/28642"
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 2 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 67 kB of PR diffs, 2 kB of issue/PR text, 2 kB of discussion (2 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.
Adds real dispatcher preflight guards before worker spawn:
HERMES_KANBAN_STRICT_DISPATCH_PREFLIGHT=1orkanban.strict_dispatch_preflight) for workspace existence/worktree checks and CAT-style task contract fieldspreflight_blockedinhermes kanban dispatch --jsonand daemon logsNotes:
mainalready skips assignees whose Hermes profile does not exist viaskipped_nonspawnable, so this PR builds on that instead of reintroducing crash-loop retriesVerification:
python3 -m py_compile hermes_cli/kanban_db.py hermes_cli/kanban.pypython3 -m pytest -q tests/hermes_cli/test_kanban_cli.py -q