Skip to content

feat(kanban): validate skill names at create + swarm time - #59691

Open
ashgupta236-commits wants to merge 1 commit into
NousResearch:mainfrom
ashgupta236-commits:kanban/validate-skill-names-at-create
Open

feat(kanban): validate skill names at create + swarm time#59691
ashgupta236-commits wants to merge 1 commit into
NousResearch:mainfrom
ashgupta236-commits:kanban/validate-skill-names-at-create

Conversation

@ashgupta236-commits

Copy link
Copy Markdown

What

Passing an unresolvable skill name via --skill (e.g. web-search when the installed path is universal/web-search) crashed the worker at spawn and burned the retry budget without doing the work. In a swarm, a bad worker skill stalled the whole graph — the verifier and synthesizer never started because the worker never completed.

Fix

Add _validate_skill_names() which resolves each name against the installed skills dirs via _find_skill() (the same resolver skill_view uses). Reject at:

  • _cmd_create--skill names checked before the card is created
  • _cmd_swarm — every worker's skills checked before the graph is built

Both paths return a clear error naming the unresolvable skill(s) + a hint pointing to the trailing path component. Import/resolver failures fall through to the existing spawn-time error as the safety net (validation is additive, never blocking on resolver hiccups).

Why

A bad skill name is a typo, not a runtime condition — it should fail fast at creation with a message, not silently crash-loop 3x and block the card. The swarm case is worse: one bad worker skill dead-locks the entire fan-out.

Verification

  • --skill web-search → rejected at create: kanban: --skill: unresolvable skill name(s): web-search + hint
  • --skill humanizer → accepted, card created and auto-dispatched normally
  • swarm with --worker researcher:bad:web-search → rejected before graph creation
  • import-clean, no syntax errors

Bad skill names passed via --skill crashed workers at spawn, burning the
retry budget without doing the work. For swarms, a bad worker skill
stalled the whole graph (verifier/synthesizer never started).

Add _validate_skill_names() which resolves each name against the
installed skills dirs via _find_skill(). Reject at _cmd_create and
_cmd_swarm with a clear error + hint naming the unresolvable skills.
Import-failures fall through to the existing spawn-time safety net.
@alt-glitch alt-glitch added type/bug Something isn't working comp/cron Cron scheduler and job management P3 Low — cosmetic, nice to have duplicate This issue or pull request already exists labels Jul 6, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #44101 (earliest open PR) -- both add create-time validation of --skill names against the installed skill registry in hermes_cli/kanban.py for the same issue (#44072), rejecting unresolvable names before the worker spawns and burns its retry budget. This PR additionally validates at swarm time (_cmd_swarm), which #44101 could fold in; maintainer to pick the canonical one. (#44092 was the earlier attempt but is closed.)

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for addressing the retry-burning failure and for adding swarm-time coverage. The problem is still present on current main: hermes_cli/kanban.py:1345 persists raw skill strings, and hermes_cli/kanban_db.py:8190-8193 forwards them to workers.

Problems

  • The helper checks the creator’s registry, but the worker runs with the assignee profile’s HERMES_HOME (hermes_cli/kanban_db.py:8096-8104). This can both accept skills absent from the worker and reject skills installed only for that worker.
  • Worker preloading resolves the original identifier through skill_view (agent/skill_commands.py:145-152), while _find_skill only matches a SKILL.md parent basename (tools/skill_manager_tool.py:585-602). The proposed slash stripping therefore is not equivalent to worker resolution, including for relative paths and plugin:skill forms documented by tools/skills_tool.py:971-973.
  • Direct creation remains unguarded in tools/kanban_tools.py:883-935 and plugins/kanban/dashboard/plugin_api.py:618-635; this PR also has no regression tests.

Suggested changes

  • Reuse or extract a profile-aware resolver that evaluates the original identifier against the assignee worker’s skill roots, then cover all intended creation surfaces with hermetic tests.
  • The existing fix(kanban): validate --skill against the worker profile's skill registry at create time #44101 discussion already documents the assignee-profile and resolver-semantic requirements; its approach is the appropriate consolidation base, with this PR’s swarm validation folded in.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cron Cron scheduler and job management duplicate This issue or pull request already exists P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants