mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-26 11:12:03 +00:00
refactor(kanban): fold worker/orchestrator skills into injected guidance (#50473)
The kanban-worker and kanban-orchestrator bundled skills existed only to be force-loaded into dispatcher-spawned workers, gated by environments:[kanban] so they wouldn't leak into normal CLI listings. That gating was fragile (the leak that #50443 patched) and the --skills auto-load was already best-effort — most workers ran without it because the bundled skill isn't present in profile-scoped skills dirs. Remove the skills entirely and promote their load-bearing content (workspace kinds, deliverable artifacts, created-card integrity, profile discovery) into KANBAN_GUIDANCE, which is already injected into every kanban worker's system prompt. Net result: every worker reliably gets the guidance, nothing can leak into a CLI/blank-slate session, and the gating machinery is gone. - agent/prompt_builder.py: promote the 4 load-bearing rules into KANBAN_GUIDANCE - hermes_cli/kanban_db.py: drop --skills kanban-worker auto-injection + _kanban_worker_skill_available probe - hermes_cli/kanban_swarm.py: drop skills=[kanban-orchestrator] on the root card - hermes_cli/kanban.py: drop kanban-init skill seeding; fix help text - delete skills/devops/kanban-{worker,orchestrator} - docs: delete the two skill pages (EN+zh), fix sidebars/catalog/kanban.md/kanban-worker-lanes.md and the video-orchestrator + codex-lane references - tests: update spawn-argv expectations; re-bound the guidance-size guard Supersedes the skill-leak half of #50443 (credit @helix4u for flagging the area).
This commit is contained in:
parent
e5e2583635
commit
84e1d31e54
32 changed files with 160 additions and 1575 deletions
|
|
@ -2703,20 +2703,17 @@ def test_build_worker_context_caps_huge_summary(kanban_home):
|
|||
conn.close()
|
||||
|
||||
|
||||
def test_default_spawn_auto_loads_kanban_worker_skill(kanban_home, monkeypatch):
|
||||
"""The dispatcher's _default_spawn must include --skills kanban-worker
|
||||
in its argv so every worker loads the skill automatically, even if
|
||||
the profile hasn't wired it into its default skills config.
|
||||
def test_default_spawn_does_not_auto_load_any_skill(kanban_home, monkeypatch):
|
||||
"""The dispatcher no longer auto-loads a bundled kanban skill.
|
||||
|
||||
The kanban lifecycle (formerly the kanban-worker/kanban-orchestrator
|
||||
skills) is now injected into every worker's system prompt via
|
||||
KANBAN_GUIDANCE, so _default_spawn must NOT append a `--skills` flag
|
||||
when the task carries no per-task skills.
|
||||
|
||||
We intercept Popen to capture the argv without actually spawning a
|
||||
hermes subprocess (which would hang trying to call an LLM).
|
||||
"""
|
||||
# Pretend the bundled kanban-worker skill resolves for this isolated
|
||||
# HERMES_HOME — the fixture creates an empty tmpdir without the
|
||||
# devops/kanban-worker tree, and _default_spawn gates the --skills
|
||||
# flag on actual resolvability.
|
||||
monkeypatch.setattr(kb, "_kanban_worker_skill_available", lambda _h: True)
|
||||
|
||||
captured = {}
|
||||
|
||||
class FakeProc:
|
||||
|
|
@ -2742,10 +2739,8 @@ def test_default_spawn_auto_loads_kanban_worker_skill(kanban_home, monkeypatch):
|
|||
conn.close()
|
||||
|
||||
cmd = captured["cmd"]
|
||||
assert "--skills" in cmd, f"spawn argv missing --skills: {cmd}"
|
||||
idx = cmd.index("--skills")
|
||||
assert cmd[idx + 1] == "kanban-worker", (
|
||||
f"expected 'kanban-worker', got {cmd[idx + 1]!r}"
|
||||
assert "--skills" not in cmd, (
|
||||
f"spawn argv should not auto-load any skill: {cmd}"
|
||||
)
|
||||
assert "--accept-hooks" in cmd, f"spawn argv missing --accept-hooks: {cmd}"
|
||||
assert cmd.index("--accept-hooks") < cmd.index("chat"), (
|
||||
|
|
@ -2985,8 +2980,7 @@ def test_create_task_skills_lists_all_toolset_typos(kanban_home):
|
|||
|
||||
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."""
|
||||
monkeypatch.setattr(kb, "_kanban_worker_skill_available", lambda _h: True)
|
||||
in declared order. No skill is auto-loaded anymore."""
|
||||
captured = {}
|
||||
|
||||
class FakeProc:
|
||||
|
|
@ -3019,10 +3013,8 @@ def test_default_spawn_appends_per_task_skills(kanban_home, monkeypatch):
|
|||
for i, tok in enumerate(cmd):
|
||||
if tok == "--skills" and i + 1 < len(cmd):
|
||||
skill_names.append(cmd[i + 1])
|
||||
# kanban-worker first (built-in), then per-task extras in order.
|
||||
assert skill_names[0] == "kanban-worker", skill_names
|
||||
assert "translation" in skill_names
|
||||
assert "github-code-review" in skill_names
|
||||
# Only the per-task skills, in declared order — nothing auto-loaded.
|
||||
assert skill_names == ["translation", "github-code-review"], skill_names
|
||||
# --skills must appear BEFORE the `chat` subcommand so argparse
|
||||
# attaches them to the top-level parser, not the subcommand.
|
||||
chat_idx = cmd.index("chat")
|
||||
|
|
@ -3034,9 +3026,9 @@ def test_default_spawn_appends_per_task_skills(kanban_home, monkeypatch):
|
|||
)
|
||||
|
||||
|
||||
def test_default_spawn_dedupes_kanban_worker_from_task_skills(kanban_home, monkeypatch):
|
||||
"""If a task explicitly lists 'kanban-worker', we don't double-pass it."""
|
||||
monkeypatch.setattr(kb, "_kanban_worker_skill_available", lambda _h: True)
|
||||
def test_default_spawn_passes_task_skills_verbatim(kanban_home, monkeypatch):
|
||||
"""Per-task skills are passed through verbatim — there is no built-in
|
||||
kanban skill to dedupe against anymore."""
|
||||
captured = {}
|
||||
|
||||
class FakeProc:
|
||||
|
|
@ -3052,7 +3044,7 @@ def test_default_spawn_dedupes_kanban_worker_from_task_skills(kanban_home, monke
|
|||
try:
|
||||
tid = kb.create_task(
|
||||
conn, title="dup", assignee="x",
|
||||
skills=["kanban-worker", "translation"],
|
||||
skills=["translation", "github-code-review"],
|
||||
)
|
||||
task = kb.get_task(conn, tid)
|
||||
workspace = kb.resolve_workspace(task)
|
||||
|
|
@ -3061,12 +3053,14 @@ def test_default_spawn_dedupes_kanban_worker_from_task_skills(kanban_home, monke
|
|||
conn.close()
|
||||
|
||||
cmd = captured["cmd"]
|
||||
worker_pairs = [
|
||||
i for i, tok in enumerate(cmd)
|
||||
if tok == "--skills" and i + 1 < len(cmd) and cmd[i + 1] == "kanban-worker"
|
||||
skill_names = [
|
||||
cmd[i + 1]
|
||||
for i, tok in enumerate(cmd)
|
||||
if tok == "--skills" and i + 1 < len(cmd)
|
||||
]
|
||||
assert len(worker_pairs) == 1, (
|
||||
f"kanban-worker appeared {len(worker_pairs)} times in argv: {cmd}"
|
||||
# Exactly the task's skills, once each, in order — no auto-loaded extras.
|
||||
assert skill_names == ["translation", "github-code-review"], (
|
||||
f"unexpected --skills in argv: {cmd}"
|
||||
)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -132,8 +132,6 @@ def test_spawn_sets_goal_env_only_when_enabled(kanban_home, monkeypatch):
|
|||
return _FakeProc()
|
||||
|
||||
monkeypatch.setattr("subprocess.Popen", _fake_popen)
|
||||
# Avoid the kanban-worker skill probe touching the real skills dir.
|
||||
monkeypatch.setattr(kb, "_kanban_worker_skill_available", lambda home: False)
|
||||
|
||||
with kb.connect() as conn:
|
||||
tid = kb.create_task(
|
||||
|
|
@ -162,7 +160,6 @@ def test_spawn_no_goal_env_for_plain_task(kanban_home, monkeypatch):
|
|||
return _FakeProc()
|
||||
|
||||
monkeypatch.setattr("subprocess.Popen", _fake_popen)
|
||||
monkeypatch.setattr(kb, "_kanban_worker_skill_available", lambda home: False)
|
||||
|
||||
with kb.connect() as conn:
|
||||
tid = kb.create_task(conn, title="plain", assignee="default")
|
||||
|
|
|
|||
|
|
@ -1224,8 +1224,16 @@ def test_kanban_guidance_in_worker_prompt(monkeypatch, tmp_path):
|
|||
|
||||
|
||||
def test_kanban_guidance_prompt_size_bounded(monkeypatch, tmp_path):
|
||||
"""Sanity: the guidance block is under 4 KB so it doesn't blow
|
||||
up the cached prompt."""
|
||||
"""Sanity: the guidance block stays lean so it doesn't blow up the
|
||||
cached prompt.
|
||||
|
||||
The ceiling guards against unbounded growth, not against any growth.
|
||||
The block absorbed the load-bearing worker/orchestrator reference
|
||||
details (workspace kinds, deliverable artifacts, created-card claims,
|
||||
profile discovery) when the standalone kanban-worker / kanban-orchestrator
|
||||
skills were removed and folded into this always-injected guidance, so the
|
||||
ceiling is sized to fit that content with a little headroom.
|
||||
"""
|
||||
monkeypatch.setenv("HERMES_KANBAN_TASK", "t_fake")
|
||||
home = tmp_path / ".hermes"
|
||||
home.mkdir()
|
||||
|
|
@ -1234,7 +1242,7 @@ def test_kanban_guidance_prompt_size_bounded(monkeypatch, tmp_path):
|
|||
monkeypatch.setattr(_P, "home", lambda: tmp_path)
|
||||
|
||||
from agent.prompt_builder import KANBAN_GUIDANCE
|
||||
assert 1_500 < len(KANBAN_GUIDANCE) < 4_096, (
|
||||
assert 1_500 < len(KANBAN_GUIDANCE) < 5_500, (
|
||||
f"KANBAN_GUIDANCE is {len(KANBAN_GUIDANCE)} chars — too short (missing?) or too long"
|
||||
)
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue