mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-04 07:31:58 +00:00
fix(kanban): CLI dispatch honors max_in_progress/max_spawn from config; swap missing 'avoid-ai-writing' skill for bundled humanizer (#33488, #29415) (#34337)
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).
This commit is contained in:
parent
8cf6b3da9d
commit
69b74c15a3
3 changed files with 181 additions and 14 deletions
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
150
tests/hermes_cli/test_kanban_cli_dispatch_passthrough.py
Normal file
150
tests/hermes_cli/test_kanban_cli_dispatch_passthrough.py
Normal file
|
|
@ -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"
|
||||
)
|
||||
Loading…
Add table
Add a link
Reference in a new issue