Skip to content

fix: load symlinked skill slash commands#27662

Open
alanxue98 wants to merge 1 commit into
NousResearch:mainfrom
alanxue98:fix/symlinked-skill-slash-load
Open

fix: load symlinked skill slash commands#27662
alanxue98 wants to merge 1 commit into
NousResearch:mainfrom
alanxue98:fix/symlinked-skill-slash-load

Conversation

@alanxue98

Copy link
Copy Markdown

Summary

  • Fix slash skill loading when an installed skill directory under ~/.hermes/skills is a symlink to a shared skills source
  • Normalize absolute slash-command skill dirs lexically against trusted roots before resolving symlinks
  • Avoid false-positive trusted-directory warnings for symlinked installed skills

Test Plan

  • /Users/alanxue/.hermes/hermes-agent/venv/bin/python -m pytest tests/agent/test_skill_commands.py -q
  • /Users/alanxue/.hermes/hermes-agent/venv/bin/python -m pytest tests/tools/test_skills_tool.py -q

Notes

This covers setups that share skills across harnesses, e.g. ~/.hermes/skills/workflow/resume-session -> ~/.claude/skills/resume-session, where the slash command was visible but invocation returned no loaded skill message because skill_view rejected the absolute resolved path.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent loop, run_agent.py, prompt builder tool/skills Skills system (list, view, manage) labels May 17, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Related symlink skill PRs (all still open, overlapping scope):

This PR takes a narrower approach: fixes path normalization/trust-check in _load_skill_payload specifically for slash-command loading, which the broader PRs may not fully address.

@fa1k3

fa1k3 commented Jun 5, 2026

Copy link
Copy Markdown

FWIW: on current main the agent/skill_commands.py portion of this PR looks already covered by an equivalent trusted-roots normalization in _load_skill_payload, so that hunk now duplicates/conflicts. The tools/skills_tool.py change (lexical-then-resolved trusted-dir check) is still needed and applies cleanly — might be worth rebasing this PR down to just that part plus the tests. Thanks for it: it resolved silent "outside the trusted skills directory" warnings for symlinked profile skill dirs in my setup.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused symlinked-skill fix. I checked this against current main.

Problems

  • The agent/skill_commands.py part is now already covered on main: _load_skill_payload prefers a lexical path under trusted roots before resolving symlinks at agent/skill_commands.py:72, added in ff078738ea0108548fc9c147140942fbeab7c833. This is why the full PR currently conflicts there.
  • The tools-side bug still appears real: tools/skills_tool.py:1117 checks only skill_md.resolve().relative_to(_td) before warning, so a skill reached through a symlink under SKILLS_DIR can still resolve outside SKILLS_DIR.resolve() and get the false “outside the trusted skills directory” warning.

Suggested changes

  • Salvage just the tools/skills_tool.py lexical-then-resolved trusted-dir check plus the tests/tools/test_skills_tool.py warning regression test.
  • Drop the now-duplicated agent/skill_commands.py hunk; current main already has equivalent slash-command loading coverage.

Automated hermes-sweeper review; humans make the final merge call.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent loop, run_agent.py, prompt builder P2 Medium — degraded but workaround exists 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.

4 participants