From 69b74c15a324fcac460b5a143e5662036dae6387 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Thu, 28 May 2026 21:00:46 -0700 Subject: [PATCH] fix(kanban): CLI dispatch honors max_in_progress/max_spawn from config; swap missing 'avoid-ai-writing' skill for bundled humanizer (#33488, #29415) (#34337) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two small bugs in the kanban dispatcher's CLI surface that were silently degrading two distinct workflows. Bundled because the test files and the surrounding code surface overlap. ## #33488: hermes kanban dispatch ignored kanban.max_in_progress / max_spawn The CLI wrapper in hermes_cli/kanban.py:_cmd_dispatch only passed default_assignee and max_in_progress_per_profile through to dispatch_once. The global concurrency cap (kanban.max_in_progress) and the per-tick spawn limit (kanban.max_spawn) were silently dropped, so operators using 'hermes kanban dispatch' as a one-shot or in a custom loop couldn't reach either cap from config — only the gateway embedded dispatcher honored them. Fix: read both keys from config in the same coerce-positive-int helper that already handled max_in_progress_per_profile. CLI --max still wins over config kanban.max_spawn when both are present (explicit operator signal beats default), but absent --max falls back to config. ## #29415: synthesizer crashed in retry loop on missing skill hermes_cli/kanban_swarm.py:212 hardcoded skills=['avoid-ai-writing'], a skill that doesn't exist in the bundled skills/ directory or any registered hub source. Every synthesizer worker spawn failed at CLI startup with 'Unknown skill(s): avoid-ai-writing' before the agent loop even started — the dispatcher retried up to failure_limit (default 2), then auto-blocked the task, then dependency rules could re-promote it, looping forever until manual intervention. Fix: replace with 'humanizer' which is bundled at skills/creative/humanizer/SKILL.md (description: 'Humanize text: strip AI-isms and add real voice'). That's the obvious intent behind the 'avoid-ai-writing' name, and the skill is platform-portable (linux/macos/windows) so it works on every supported runtime. ## Tests tests/hermes_cli/test_kanban_cli_dispatch_passthrough.py — 4 cases: - CLI passes max_in_progress / max_spawn / default_assignee / max_in_progress_per_profile from config to dispatch_once - CLI --max flag overrides config kanban.max_spawn - Invalid cap values (0, -1, 'abc', '1.5') silently fall through to None - kanban_swarm.py no longer references 'avoid-ai-writing' AND the replacement 'humanizer' skill exists at the expected on-disk path Kanban suite: 468/468 pass (was 464; +4 new regression tests). --- hermes_cli/kanban.py | 43 +++-- hermes_cli/kanban_swarm.py | 2 +- .../test_kanban_cli_dispatch_passthrough.py | 150 ++++++++++++++++++ 3 files changed, 181 insertions(+), 14 deletions(-) create mode 100644 tests/hermes_cli/test_kanban_cli_dispatch_passthrough.py diff --git a/hermes_cli/kanban.py b/hermes_cli/kanban.py index 3f5bfab68cf..8d52714c224 100644 --- a/hermes_cli/kanban.py +++ b/hermes_cli/kanban.py @@ -2088,31 +2088,48 @@ def _cmd_tail(args: argparse.Namespace) -> int: def _cmd_dispatch(args: argparse.Namespace) -> int: # Honour kanban.default_assignee as the fallback for unassigned ready - # tasks (#27145) and kanban.max_in_progress_per_profile as the - # per-profile concurrency cap (#21582). Same semantics as the - # gateway dispatch path. + # tasks (#27145), kanban.max_in_progress as the global concurrency cap + # (#33488), kanban.max_in_progress_per_profile as the per-profile + # cap (#21582), and kanban.max_spawn as the per-tick spawn limit + # (#28805). Same semantics as the gateway dispatch path so behavior + # matches whether the user runs the CLI directly or relies on the + # gateway-embedded dispatcher. try: from hermes_cli.config import load_config _cfg = load_config() _kanban_cfg = _cfg.get("kanban", {}) if isinstance(_cfg, dict) else {} default_assignee = (_kanban_cfg.get("default_assignee") or "").strip() or None - _raw_per_profile = _kanban_cfg.get("max_in_progress_per_profile", None) - try: - max_in_progress_per_profile = ( - int(_raw_per_profile) if _raw_per_profile is not None else None - ) - if max_in_progress_per_profile is not None and max_in_progress_per_profile < 1: - max_in_progress_per_profile = None - except (TypeError, ValueError): - max_in_progress_per_profile = None + + def _coerce_positive_int(value): + if value is None: + return None + try: + ival = int(value) + except (TypeError, ValueError): + return None + return ival if ival >= 1 else None + + max_in_progress_per_profile = _coerce_positive_int( + _kanban_cfg.get("max_in_progress_per_profile") + ) + max_in_progress = _coerce_positive_int(_kanban_cfg.get("max_in_progress")) + # CLI --max overrides config kanban.max_spawn when both are present; + # CLI is the more explicit signal so it wins. + cli_max = getattr(args, "max", None) + max_spawn = cli_max if cli_max is not None else _coerce_positive_int( + _kanban_cfg.get("max_spawn") + ) except Exception: default_assignee = None max_in_progress_per_profile = None + max_in_progress = None + max_spawn = getattr(args, "max", None) with kb.connect_closing() as conn: res = kb.dispatch_once( conn, dry_run=args.dry_run, - max_spawn=args.max, + max_spawn=max_spawn, + max_in_progress=max_in_progress, failure_limit=getattr(args, "failure_limit", kb.DEFAULT_SPAWN_FAILURE_LIMIT), default_assignee=default_assignee, max_in_progress_per_profile=max_in_progress_per_profile, diff --git a/hermes_cli/kanban_swarm.py b/hermes_cli/kanban_swarm.py index 2b0fa0b9e98..fe47a4c7713 100644 --- a/hermes_cli/kanban_swarm.py +++ b/hermes_cli/kanban_swarm.py @@ -209,7 +209,7 @@ def create_swarm( priority=priority, workspace_kind=workspace_kind, workspace_path=workspace_path, - skills=["avoid-ai-writing"], + skills=["humanizer"], ) created = SwarmCreated(root, worker_ids, verifier, synthesizer) diff --git a/tests/hermes_cli/test_kanban_cli_dispatch_passthrough.py b/tests/hermes_cli/test_kanban_cli_dispatch_passthrough.py new file mode 100644 index 00000000000..8bb75fe6292 --- /dev/null +++ b/tests/hermes_cli/test_kanban_cli_dispatch_passthrough.py @@ -0,0 +1,150 @@ +"""Regression tests for #33488 (CLI max_in_progress / max_spawn / per-profile +config passthrough) and #29415 (kanban_swarm humanizer skill ref). + +These two fixes are bundled because they're both small, both touch the +kanban dispatcher's CLI surface, and they each guard against a silent +operator footgun that only manifests in long-running setups. +""" +from __future__ import annotations + +import argparse +import os +import sys +import tempfile +from types import SimpleNamespace +from unittest.mock import MagicMock, patch + +import pytest + + +@pytest.fixture() +def isolated_kanban_home(monkeypatch): + """Spin up a fresh HERMES_HOME with a clean kanban DB.""" + test_home = tempfile.mkdtemp(prefix="kanban_cli_passthrough_") + os.makedirs(os.path.join(test_home, "profiles", "default"), exist_ok=True) + monkeypatch.setenv("HERMES_HOME", test_home) + for mod in list(sys.modules.keys()): + if mod.startswith("hermes_cli") or mod.startswith("hermes_state") or mod == "hermes_constants": + del sys.modules[mod] + yield test_home + + +def test_cli_dispatch_passes_max_in_progress_from_config(isolated_kanban_home, monkeypatch): + """#33488: hermes kanban dispatch must pass kanban.max_in_progress from + config to dispatch_once. Without this, the global concurrency cap is + unreachable from the CLI even though it works from the gateway.""" + from hermes_cli import kanban as kb_cli + from hermes_cli import kanban_db + + # Configure max_in_progress in the loaded config. + fake_config = { + "kanban": { + "max_in_progress": 3, + "max_spawn": 5, + "default_assignee": "default", + "max_in_progress_per_profile": 2, + } + } + monkeypatch.setattr( + "hermes_cli.config.load_config", lambda: fake_config + ) + + captured = {} + + def fake_dispatch_once(conn, **kwargs): + captured.update(kwargs) + return kanban_db.DispatchResult() + + monkeypatch.setattr(kanban_db, "dispatch_once", fake_dispatch_once) + + args = argparse.Namespace(dry_run=True, max=None, failure_limit=2, json=False) + kb_cli._cmd_dispatch(args) + + # Every config value must have reached dispatch_once. + assert captured.get("max_in_progress") == 3, ( + f"CLI must pass kanban.max_in_progress from config; got {captured.get('max_in_progress')!r}" + ) + assert captured.get("max_spawn") == 5, ( + f"CLI must pass kanban.max_spawn from config when --max is not provided; got {captured.get('max_spawn')!r}" + ) + assert captured.get("default_assignee") == "default" + assert captured.get("max_in_progress_per_profile") == 2 + + +def test_cli_max_flag_overrides_config_max_spawn(isolated_kanban_home, monkeypatch): + """--max on the CLI takes precedence over kanban.max_spawn in config. + The CLI flag is the explicit operator signal; config is the default.""" + from hermes_cli import kanban as kb_cli + from hermes_cli import kanban_db + + fake_config = {"kanban": {"max_spawn": 10}} + monkeypatch.setattr("hermes_cli.config.load_config", lambda: fake_config) + + captured = {} + monkeypatch.setattr( + kanban_db, "dispatch_once", + lambda conn, **kw: (captured.update(kw), kanban_db.DispatchResult())[1], + ) + + args = argparse.Namespace(dry_run=True, max=2, failure_limit=2, json=False) + kb_cli._cmd_dispatch(args) + + assert captured.get("max_spawn") == 2, ( + f"CLI --max=2 must override config kanban.max_spawn=10; got {captured.get('max_spawn')!r}" + ) + + +def test_cli_invalid_max_in_progress_silently_disables(isolated_kanban_home, monkeypatch): + """Invalid kanban.max_in_progress values (0, negative, non-int) should + silently fall through to None — no crash, no surprise behavior.""" + from hermes_cli import kanban as kb_cli + from hermes_cli import kanban_db + + for bad_val in (0, -1, "abc", "1.5"): + fake_config = {"kanban": {"max_in_progress": bad_val}} + monkeypatch.setattr("hermes_cli.config.load_config", lambda: fake_config) + captured = {} + monkeypatch.setattr( + kanban_db, "dispatch_once", + lambda conn, **kw: (captured.update(kw), kanban_db.DispatchResult())[1], + ) + args = argparse.Namespace(dry_run=True, max=None, failure_limit=2, json=False) + kb_cli._cmd_dispatch(args) + assert captured.get("max_in_progress") is None, ( + f"invalid max_in_progress={bad_val!r} should fall through to None, " + f"got {captured.get('max_in_progress')!r}" + ) + + +def test_kanban_swarm_uses_existing_humanizer_skill(): + """#29415: kanban_swarm.py used to hardcode skills=['avoid-ai-writing'], + a skill that doesn't exist in any registry — synthesizer workers + crashed with 'Unknown skill(s): avoid-ai-writing' on every retry. + + Verify the synthesizer card now uses the bundled 'humanizer' skill + which actually exists at skills/creative/humanizer/SKILL.md.""" + import pathlib + + swarm_path = ( + pathlib.Path(__file__).resolve().parent.parent.parent + / "hermes_cli" / "kanban_swarm.py" + ) + src = swarm_path.read_text() + assert "avoid-ai-writing" not in src, ( + "kanban_swarm.py must not reference 'avoid-ai-writing' — that " + "skill doesn't exist in any registry, crashing synthesizers (#29415)" + ) + assert '"humanizer"' in src, ( + "kanban_swarm.py should use the bundled 'humanizer' skill for " + "synthesizer cards (the original intent of 'avoid-ai-writing')" + ) + + # And the replacement skill must actually exist on disk. + skills_root = ( + pathlib.Path(__file__).resolve().parent.parent.parent / "skills" + ) + humanizer_path = skills_root / "creative" / "humanizer" / "SKILL.md" + assert humanizer_path.is_file(), ( + f"humanizer skill missing at {humanizer_path}; the kanban_swarm fix " + "for #29415 requires this bundled skill to exist" + )