Skip to content

fix(skills): treat slash-command load failures explicitly - #11380

Closed
yeyitech wants to merge 1 commit into
NousResearch:mainfrom
yeyitech:fix/issue-11200-skill-load-failure
Closed

fix(skills): treat slash-command load failures explicitly#11380
yeyitech wants to merge 1 commit into
NousResearch:mainfrom
yeyitech:fix/issue-11200-skill-load-failure

Conversation

@yeyitech

Copy link
Copy Markdown
Contributor

Summary

  • return an explicit SkillInvocationResult from build_skill_invocation_message() instead of a truthy placeholder string
  • update CLI, gateway, and webhook callers to handle ok, unknown_command, and load_failed outcomes explicitly
  • add regressions for post-scan skill load failures so broken skills no longer inject bogus prompt text into the agent

Closes #11200.

Testing

  • PYTHONPATH=/tmp/hermes-pytest-asyncio python -m pytest -o addopts= tests/agent/test_skill_commands.py tests/cli/test_quick_commands.py tests/cli/test_cli_plan_command.py tests/gateway/test_unknown_command.py tests/gateway/test_plan_command.py tests/gateway/test_webhook_integration.py -q

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard comp/gateway Gateway runner, session dispatch, delivery tool/skills Skills system (list, view, manage) labels Apr 25, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Related to #11408 and #14752 — all three fix the same root cause: build_skill_invocation_message() returns truthy error string on load failure, treated as success. This PR uses an explicit SkillInvocationResult dataclass approach.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the clear reproduction and the explicit-result design.

Automated hermes-sweeper review found that the behavioral bug is already fixed on current main:

  • agent/skill_commands.py:509-511 returns None when the post-scan skill payload cannot load, preventing the former truthy failure placeholder from reaching CLI/gateway callers.
  • tests/agent/test_skill_commands.py:532-538 covers that failed-load path.
  • Commit 4b17c2411ab2518fabe9d87872a03a47d3b8cfcc made this exact correction and shipped in v2026.5.28.

The typed SkillInvocationResult alternative is therefore superseded by the implementation already on main.

@teknium1 teknium1 closed this Jul 12, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jul 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists sweeper:implemented-on-main Sweeper: behavior already present on current main tool/skills Skills system (list, view, manage) type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Skill load failure is treated as successful slash-command invocation

3 participants