Skip to content

fix: resolve stale import-time HERMES_HOME paths across all call sites - #60400

Open
isheng-eqi wants to merge 9 commits into
NousResearch:mainfrom
isheng-eqi:clean/profile-aware-all
Open

fix: resolve stale import-time HERMES_HOME paths across all call sites#60400
isheng-eqi wants to merge 9 commits into
NousResearch:mainfrom
isheng-eqi:clean/profile-aware-all

Conversation

@isheng-eqi

Copy link
Copy Markdown
Contributor

What does this PR do?

#60180 introduced _skills_dir() for call-time profile-scoped skills resolution, but four sibling paths still captured get_hermes_home() at import time — causing stale-path behavior in long-lived multi-profile runtimes (Dashboard/TUI/Desktop backend, cron).

  • agent/skill_utils.py: normalize_skill_lookup_name() uses _skills_dir() instead of SKILLS_DIR
  • agent/auxiliary_client.py: _AUTH_JSON_PATH replaced with call-time _auth_json_path()
  • agent/skill_commands.py: Profile-scoped cache invalidation, _skills_dir() in scan
  • hermes_cli/web_server.py: Drop redundant module-global patching (14 lines saved)

Related Issue

Completes the #60180 profile-awareness chain (#40677, #14536).

Type of Change

  • Bug fix
  • Refactor

Changes Made

4 source files + 3 test files covering call-time resolution across the multi-profile runtime surface.

How to Test

  1. Start a multi-profile Dashboard instance
  2. Switch profiles — verify skills, auth, and commands resolve from the correct profile
  3. Verify the TUI model picker works without stale-path errors

The profile-aware skills resolution fix (NousResearch#60180) added _skills_dir() to
tools/skills_tool.py and tools/skill_manager_tool.py so that long-lived
multi-profile runtimes always resolve the live profile-scoped HERMES_HOME.
agent/skill_utils.py still referenced the module-level SKILLS_DIR constant,
which would return stale import-time paths in the same long-lived runtimes.

Replace _skills_tool.SKILLS_DIR with _skills_tool._skills_dir() in the
normalize_skill_identifier() path, matching the pattern established in
NousResearch#60180. Tests that patch SKILLS_DIR are unaffected — _skills_dir() falls
back to the patched value when it differs from _SKILLS_DIR_AT_IMPORT.

Refs: NousResearch#60180, NousResearch#40677
…n profile scope

After NousResearch#60180, _skills_dir() resolves SKILLS_DIR at call time via the
live get_hermes_home(). The manual monkeypatching of
_skills_tool.SKILLS_DIR/HERMES_HOME and _skill_mgr.SKILLS_DIR/HERMES_HOME
in _profile_scope() is no longer necessary — set_hermes_home_override()
alone suffices. Remove the redundant patching, drop the unused module
imports, and update stale docstrings in both _profile_scope() and
_config_profile_scope().

Refs: NousResearch#60180, NousResearch#40677
…-lived runtimes

get_skill_commands() caches scanned skills in module-level _skill_commands
and only invalidates when the active platform changes (NousResearch#14536). In
long-lived multi-profile runtimes (Dashboard/TUI/Desktop backend), the
cache is not invalidated when the active profile changes, causing the
wrong profile's skills to appear in the slash-command map.

Add _skill_commands_skills_dir tracking alongside the existing platform
check. On each get_skill_commands() call, compare the cached skills_dir
against the current _skills_dir() — if they differ (profile change),
rescan. Also update scan_skill_commands() to use _skills_dir() (call-time
resolution, NousResearch#60180) instead of the module-level SKILLS_DIR constant.

Refs: NousResearch#60180, NousResearch#40677, NousResearch#14536
… runtimes

Module-level _AUTH_JSON_PATH captures get_hermes_home() at import time.
In long-lived multi-profile runtimes (Dashboard/TUI/Desktop backend),
this path becomes stale when the active profile changes, causing auth
token reads from the wrong profile (NousResearch#40677).

Add _auth_json_path() that resolves from the live HERMES_HOME, matching
the pattern established by _skills_dir() in NousResearch#60180.

Refs: NousResearch#60180, NousResearch#40677
@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint tool/skills Skills system (list, view, manage) area/auth Authentication, OAuth, credential pools comp/dashboard Web dashboard / control panel UI (dashboard/, landing) P3 Low — cosmetic, nice to have labels Jul 7, 2026

@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 tracing the remaining profile-scoped paths. The auth and slash-command premises are still present on current main (agent/auxiliary_client.py:1651-1653, agent/skill_commands.py:330-340), and the call-time approach matches tools/skills_tool.py:147-158.

Problems

  • tests/agent/test_skill_utils_profile_aware.py:27 passes a relative name. normalize_skill_lookup_name() returns relative inputs at agent/skill_utils.py:528-529, so this regression test never distinguishes SKILLS_DIR from _skills_dir().
  • The all-call-sites claim is incomplete: hermes_cli/commands.py:840-843 and hermes_cli/commands.py:1021-1024 still derive menu filter roots from import-time SKILLS_DIR, while the PR makes get_skill_commands() return paths from the active root. A profile-scoped menu can therefore filter out its own skills.

Suggested changes

  • Test an absolute skill path under the active profile and assert its normalized relative path.
  • Switch both menu collectors to _skills_dir() and cover the active-profile menu path.

Automated hermes-sweeper review.

from agent.skill_utils import normalize_skill_lookup_name

result = normalize_skill_lookup_name("test-skill")
# Should use profile_b (from _skills_dir()), not profile_a (stale SKILLS_DIR)

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 relative input returns before normalize_skill_lookup_name() evaluates either root. Use an absolute path under profile_b / "skills" and assert the expected relative result so the test fails with the old SKILLS_DIR implementation.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data 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

@GottZ GottZ 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.

This was generated by AI during triage.

Summary

Two PRs address stale import-time HERMES_HOME resolution. #60315 is a broad omnibus whose profile-scoping subset matches #60400, while #60400 isolates call-time auth and skills-path resolution plus profile-aware slash-command cache invalidation, but does not yet cover every affected menu path.

Related pull requests

  • #60315 [closed] duplicate — (+577/-74) — superseded omnibus: Its changes in auxiliary_client, skill_commands, skill_utils, and web_server substantially duplicate #60400, while its unrelated fixes span several additional issue areas. Although closed, it remains relevant as the earlier superset; contributor review records its other content as covered by #60285, #60264, #60273, #60317, and #60222.
  • #60400 related — (+187/-41) — salvageable but incomplete: The diff replaces stale import-time auth and skills roots with call-time resolution and invalidates the slash-command cache when the active profile changes. The keep_open review identifies two concrete gaps: the skill-utils test uses a relative input that cannot distinguish SKILLS_DIR from _skills_dir(), and hermes_cli/commands.py:840-843 plus hermes_cli/commands.py:1021-1024 still filter against the import-time root.

Duplicates

#60315 and #60400 contain essentially the same profile-scoped path changes; #60315 is the broader, already-closed superset, while #60400 is the focused surviving PR.

Suggested consolidation

Keep #60400 open with a salvage path, consistent with its keep_open review: retain the call-time auth resolution and profile-aware slash-command cache work, change the normalization regression test to use an absolute path under the active profile, and switch both hermes_cli/commands.py menu collectors to _skills_dir() with active-profile coverage. Keep #60315 closed as the superseded duplicate/superset; its unrelated portions were separately covered by #60285, #60264, #60273, #60317, and #60222.

Complex graph

flowchart LR
    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
    subgraph Dup60315 ["PRs duplicating each other"]
        P60315["PR #60315 (closed)"]
        P60400["PR #60400 (open)"]
    end
    class P60315 closed
    class P60400 open
    class P60400 target
    click P60315 "https://github.com/NousResearch/hermes-agent/pull/60315"
    click P60400 "https://github.com/NousResearch/hermes-agent/pull/60400"
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 2 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 57 kB of PR diffs, 3 kB of issue/PR text, 1 kB of discussion (2 comments), 0 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/auth Authentication, OAuth, credential pools comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/dashboard Web dashboard / control panel UI (dashboard/, landing) 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 sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data 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