Skip to content

fix(skills): resolve install path against resolved skills dir (Windows junction) - #76885

Open
bingu7 wants to merge 1 commit into
NousResearch:mainfrom
bingu7:fix/skills-hub-junction-path
Open

bingu7 wants to merge 1 commit into
NousResearch:mainfrom
bingu7:fix/skills-hub-junction-path

Conversation

@bingu7

@bingu7 bingu7 commented Aug 2, 2026

Copy link
Copy Markdown

Summary

Fixes skills-hub installs failing with:

Installation blocked: 'D:\hermes\skills\web-pentest' is not in the subpath of 'C:\Users\...\hermes\skills' OR one path is relative and the other is absolute.

Root cause: When HERMES_HOME lives on a Windows directory junction (e.g. C:\Users\<user>\AppData\Local\hermesD:\hermes), _resolve_lock_install_path() returns the physical path (D:\hermes\skills\<skill>) after .resolve(), but the subsequent relative_to() calls compared it against the junction-form skills dir (C:\Users\<user>\AppData\Local\hermes\skills). The drive-letter mismatch makes Path.relative_to() raise ValueError, blocking every skills-hub install for junction/symlinked-home users.

Fix: Resolve the base dir too (_skills_dir().resolve() / SKILLS_DIR.resolve()), so both sides compare physical paths. Applies to both:

  • tools/skills_hub.py — the lock-file install_path record (install_from_quarantine)
  • hermes_cli/skills_hub.py — the post-install display path

Test Plan

  • On a junction-based HERMES_HOME (repro environment): hermes skills install official/security/web-pentest previously failed for every skill; now installs successfully (verified with 7 official skills).
  • scripts/run_tests.sh tests/tools/test_skills_hub.py tests/hermes_cli/test_skills_hub.py: 57 passed, 2 pre-existing failures (binary-asset KeyError, unrelated to this change — fails on clean main too).

…ows junctions

When HERMES_HOME lives on a directory junction (e.g. C:\Users\...\hermes
→ D:\hermes), _resolve_lock_install_path() returns the physical path
(D:\hermes\skills\<skill>) after .resolve(), but the subsequent
relative_to() call compared it against the junction-form skills dir
(C:\Users\...\hermes\skills). The drive-letter mismatch makes
Path.relative_to() raise ValueError, blocking every skills-hub install.

Fix by resolving the base dir too, so both sides compare physical paths.
Applies to both the lock-file install_path record and the post-install
display path.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tracing this to the resolved install target versus logical Skills Hub root. Current main still returns a resolved target from tools/skills_hub.py:265-291, then records it relative to unresolved _skills_dir() at tools/skills_hub.py:3824; the CLI repeats the mismatch at hermes_cli/skills_hub.py:740. The two changed bases address those exact paths.

Problems

  • The diff has no regression coverage. The existing install E2E at tests/tools/test_skill_bundle_provenance.py:141-165 uses a direct temporary HERMES_HOME, so it cannot exercise a symlink/junction-root mismatch.

Suggested changes

  • Add an E2E test using a logical symlink/junction HERMES_HOME pointing to a physical temporary home; assert install completion, a relative lock-file install_path, and successful CLI output.

Automated hermes-sweeper review.

Comment thread tools/skills_hub.py
scan_verdict=scan_result.verdict,
skill_hash=content_hash(install_dir),
install_path=str(install_dir.relative_to(_skills_dir())),
install_path=str(install_dir.relative_to(_skills_dir().resolve())),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add an E2E regression test with HERMES_HOME pointing through a symlink/junction to a physical temporary home. Assert installation completes and the lock entry remains relative; the current E2E uses a direct home path and cannot cover this resolved-versus-logical-base mismatch.

@teknium1 teknium1 added sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users area/install-update Installer, updater, packaging, wheels, doctor labels Aug 2, 2026
@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 tool/skills Skills system (list, view, manage) platform/windows Native Windows-specific behavior or breakage labels Aug 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/install-update Installer, updater, packaging, wheels, doctor comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists platform/windows Native Windows-specific behavior or breakage sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows 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.

3 participants