From 1f5983c4c8cf9227266fcbff88ca1c91bbb4d344 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Sun, 10 May 2026 08:40:44 -0700 Subject: [PATCH] feat(kanban): aggregate all toolset-name typos in skills before raising MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the previous commit's toolset-vs-skill validation. The contributor's fix raises ValueError on the first toolset name found in the skills list. That works for one mistake, but agents that confuse skills with toolsets usually pass several at once (`skills=["web", "browser", "terminal"]`) — and serial-correcting one per failure round-trip wastes tokens. Collect all toolset-shaped entries first, then raise once with the full list. The error message is also slightly clearer: 'web', 'browser', 'terminal' are toolset names, not skill name(s). Put toolsets in the assignee profile's `toolsets:` config instead of per-task skills. Skills are named skill bundles (e.g. `kanban-worker`, `blogwatcher`); toolsets are runtime capabilities (e.g. `web`, `browser`, `terminal`). vs. the previous "the assignee profile's toolsets" — explicitly naming the YAML key (`toolsets:`) and giving concrete examples in both categories closes the conceptual gap that produced the bug to begin with. Adds one regression test (test_create_task_skills_lists_all_toolset_typos) covering the multi-name aggregation path. The single-typo test from the original PR still passes (the loose `match="toolset name"` matches both singular and plural forms). --- hermes_cli/kanban_db.py | 22 ++++++++++++--- .../test_kanban_core_functionality.py | 27 +++++++++++++++++++ 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/hermes_cli/kanban_db.py b/hermes_cli/kanban_db.py index dc2ec507e78..86b399a9671 100644 --- a/hermes_cli/kanban_db.py +++ b/hermes_cli/kanban_db.py @@ -1275,6 +1275,12 @@ def create_task( if skills is not None: cleaned: list[str] = [] seen: set[str] = set() + # Collect all toolset-name confusions up front so the user sees the + # whole list at once. Raising on the first hit is friendly when the + # input has one mistake, but agents that confuse skills with toolsets + # usually pass several at once (`skills=["web", "browser", "terminal"]`) + # and serial-correcting one per failure round-trips wastes tokens. + toolset_typos: list[str] = [] for s in skills: if not s: continue @@ -1287,14 +1293,22 @@ def create_task( f"(pass a list of separate names instead of a comma-joined string)" ) if name.casefold() in KNOWN_TOOLSET_NAMES: - raise ValueError( - f"{name!r} is a toolset name, not a skill name. " - "Put it in the assignee profile's toolsets instead of task skills." - ) + toolset_typos.append(name) + continue if name in seen: continue seen.add(name) cleaned.append(name) + if toolset_typos: + quoted = ", ".join(repr(n) for n in toolset_typos) + noun = "is a toolset name" if len(toolset_typos) == 1 else "are toolset names" + raise ValueError( + f"{quoted} {noun}, not skill name(s). " + "Put toolsets in the assignee profile's `toolsets:` config " + "instead of per-task skills. Skills are named skill bundles " + "(e.g. `kanban-worker`, `blogwatcher`); toolsets are runtime " + "capabilities (e.g. `web`, `browser`, `terminal`)." + ) skills_list = cleaned # Idempotency check — return the existing task instead of creating a diff --git a/tests/hermes_cli/test_kanban_core_functionality.py b/tests/hermes_cli/test_kanban_core_functionality.py index ed5172c82de..c5ec3fa4ed2 100644 --- a/tests/hermes_cli/test_kanban_core_functionality.py +++ b/tests/hermes_cli/test_kanban_core_functionality.py @@ -2706,6 +2706,33 @@ def test_create_task_skills_rejects_toolset_names(kanban_home): conn.close() +def test_create_task_skills_lists_all_toolset_typos(kanban_home): + """When several toolset names are passed, the error names every one. + + Agents that confuse skills with toolsets usually pass several at once + (``skills=["web", "browser", "terminal"]``). Listing only the first + mistake forces serial fix-then-retry; listing all of them lets the + caller correct in one round-trip. + """ + conn = kb.connect() + try: + with pytest.raises(ValueError) as exc_info: + kb.create_task( + conn, + title="three bad", + assignee="x", + skills=["web", "browser", "terminal"], + ) + msg = str(exc_info.value) + assert "'web'" in msg + assert "'browser'" in msg + assert "'terminal'" in msg + # Plural noun form when multiple toolsets are flagged. + assert "are toolset names" in msg + finally: + conn.close() + + def test_default_spawn_appends_per_task_skills(kanban_home, monkeypatch): """Dispatcher argv must carry one `--skills X` pair per task skill, in addition to the built-in kanban-worker."""