Skip to content

fix: resolve _skills_dir() for symlinked HERMES_HOME install paths (#64953) - #65050

Open
webtecnica wants to merge 1 commit into
NousResearch:mainfrom
webtecnica:fix/64953-symlink-install
Open

fix: resolve _skills_dir() for symlinked HERMES_HOME install paths (#64953)#65050
webtecnica wants to merge 1 commit into
NousResearch:mainfrom
webtecnica:fix/64953-symlink-install

Conversation

@webtecnica

Copy link
Copy Markdown
Contributor

Prevents false 'Installation blocked' error when HERMES_HOME is a symlink. Resolves _skills_dir() before relative_to() so resolved and unresolved paths match.\n\nFixes #64953.

@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard tool/skills Skills system (list, view, manage) sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades P2 Medium — degraded but workaround exists duplicate This issue or pull request already exists labels Jul 15, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #49885 — this is the identical one-line _skills_dir().resolve() fix at the same single install_from_quarantine site as the earliest open canonical. The both-sites superset PRs (#64954, #53409, #53493) also fix the hermes_cli/skills_hub.py Installed: line; maintainer picks between the narrow and superset fixes.

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review Summary

Verdict: Approved

Looks Good

  • Tiny fix: resolves _skills_dir() for symlinked HERMES_HOME install paths (#64953)
  • 1 addition, 1 deletion — single-line fix, trivially correct

Reviewed by Hermes Agent

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for isolating the resolved/unresolved path mismatch.

Problems

  • _resolve_lock_install_path() returns target.resolve() at tools/skills_hub.py:288-291, and the proposed lock-record change correctly aligns with that. However, the normal CLI success path still performs the same unresolved-root comparison at hermes_cli/skills_hub.py:730-731. After the lock entry succeeds, a symlinked HERMES_HOME can still raise while rendering Installed:.
  • This PR changes only tools/skills_hub.py; add a regression test covering an actual symlinked HERMES_HOME and asserting the lock entry plus successful CLI completion.

Suggested changes

  • Resolve the SKILLS_DIR root at hermes_cli/skills_hub.py:731 before relative_to().
  • Add the symlinked-root regression and preserve an escaping-path assertion for the existing containment boundary.

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
Contributor

Choose a reason for hiding this comment

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

This fixes the lock entry, but the same resolved/unresolved comparison remains in the normal CLI success path at hermes_cli/skills_hub.py:731 (install_dir.relative_to(SKILLS_DIR)). Please resolve that root too; otherwise a symlinked HERMES_HOME can still raise after the lock entry is recorded.

@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown

This was generated by AI during triage.

Summary

18 PRs address or reference this issue complex; seven modify the resolved-install-path versus unresolved-skills-root mismatch behind #53403/#64953, while the others cover destination symlinks, CLI compatibility, skill metadata, or slash-command documentation. #64954 is the recorded best existing fix for the symlink-root cause because its diff covers both lock recording and the CLI success display on the current code shape, whereas #65050 changes only lock recording.

Related pull requests

Duplicates

For the symlink-root cause, #35630, #53409, #53493, and #64954 overlap at both production sites; #49885 and #53541 are older backend-only variants, and #65050 is the current-main backend-only subset of #64954. Separately, #23251 duplicates #21739; #50608, #50613, #62933, and #63451 substantially duplicate the correction delivered by #70511; #25501 overlaps the CLI-compatibility part of #54444, and #53447 overlaps its here-now part.

Suggested consolidation

Keep #64954 open with a salvage path as the canonical #53403/#64953 branch: preserve its current-main two-site changes and symlink/escape regressions, consistent with the maintainer-bot keep_open verdict and recorded best-fix findings. Close #65050 as a backend-only duplicate of #64954, and likewise close #35630, #49885, #53409, #53493, and #53541 as superseded variants because their visible diffs use obsolete path APIs, omit one affected call site, lack equivalent coverage, or bundle unrelated work; keep #53447, #54444, and #71295 open for their independent documented salvage paths.

Complex graph

flowchart TD
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I53403(["issue #53403 (open)"])
    I64953(["issue #64953 (open)"])
    subgraph Dup35630 ["PRs duplicating each other"]
        P35630["PR #35630 (open)"]
        P49885["PR #49885 (open)"]
        P53409["PR #53409 (open)"]
        P53493["PR #53493 (open)"]
        P53541["PR #53541 (open)"]
        P64954["PR #64954 (open)"]
        P65050["PR #65050 (open)"]
    end
    P65050 -.->|partial| I53403
    P65050 -.->|partial| I64953
    class I53403 open
    class I64953 open
    class P35630 open
    class P49885 open
    class P53409 open
    class P53493 open
    class P53541 open
    class P64954 open
    class P65050 open
    class P64954 best
    class P64954 best
    class P65050 target
    click I53403 "https://github.com/NousResearch/hermes-agent/issues/53403"
    click I64953 "https://github.com/NousResearch/hermes-agent/issues/64953"
    click P35630 "https://github.com/NousResearch/hermes-agent/pull/35630"
    click P49885 "https://github.com/NousResearch/hermes-agent/pull/49885"
    click P53409 "https://github.com/NousResearch/hermes-agent/pull/53409"
    click P53493 "https://github.com/NousResearch/hermes-agent/pull/53493"
    click P53541 "https://github.com/NousResearch/hermes-agent/pull/53541"
    click P64954 "https://github.com/NousResearch/hermes-agent/pull/64954"
    click P65050 "https://github.com/NousResearch/hermes-agent/pull/65050"
Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 18 pull requests and 7 issues in this complex. Each diff was read against this issue; Assessment working set: 97 kB of PR diffs, 49 kB of issue/PR text, 35 kB of discussion (51 comments), 36 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

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 duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists 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 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.

5 participants