fix(skills): resolve SKILLS_DIR at call time, not at module load - #59040
Closed
Ahmett101 wants to merge 1 commit into
Closed
fix(skills): resolve SKILLS_DIR at call time, not at module load#59040Ahmett101 wants to merge 1 commit into
Ahmett101 wants to merge 1 commit into
Conversation
Contributor
|
Closing as duplicate: the same call-time skills-dir resolution fix was submitted earlier in #56689 by @jplew (Jul 1) and has now been merged via PR #60180 (#60180), which covers |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
tools/skills_tool._find_all_skills()was resolving[SKILLS_DIR](file:///home/pstk/.hermes/profiles/merlin/skills)once at module import via[SKILLS_DIR = HERMES_HOME / "skills"](file:///home/pstk/.hermes/profiles/merlin/skills). A long-running gateway process (systemd unit, multi-profile setup) readsHERMES_HOMEonce at boot — when the env drift's between launch and tool calls later, the function silently returns a stale skill index that diverges from a fresh Python subprocess (80 vs 88 for a profile with 88 on-disk skills, spanning 8 entire categories:meta,hermes-config,content-creation,scheduled,thinking,tools,transfer,video).get_skill_commands()(which the report confirms returns all 88) readsHERMES_HOME / "skills"per call. Converge to the same pattern in_find_all_skills()so the count from the gateway matches the count from any fresh subprocess.The fix preserves backward compatibility for tests / callers that
patchthe module-levelSKILLS_DIR: when patched, that override is honoured. When not patched (production runtime), the function re-readsHERMES_HOMEviaget_hermes_home()and rescans the current home root.Changes
[tools/skills_tool.py](file:///data/data/com.termux/files/home/.hermes/hermes-agent/tools/skills_tool.py)—_find_all_skills()resolvesSKILLS_DIRper call viaget_hermes_home(); falls back to the module-levelSKILLS_DIRonly when tests have patched it.[tests/tools/test_skills_tool.py](file:///data/data/com.termux/files/home/.hermes/hermes-agent/tests/tools/test_skills_tool.py)— two regression pins:test_find_all_skills_source_re_resolves_skills_dirpins the live-resolve behaviour by source inspection (a future refactor that goes back to module-level-only read silently regresses this fix without breaking any other test).test_find_all_skills_results_use_call_time_skills_direxercises the end-to-end behaviour: swapping the module-levelSKILLS_DIRbetween two isolated home roots toggles which roots are scanned.How to Test
Risk & Impact
Low. The change only adds a per-call
get_hermes_home()lookup, which is already the pathget_skill_commands()and most other adapter entry-points take. The path is a single stat for a non-existent or shallow directory — negligible cost relative to the existing disk walk overiter_skill_index_files(...)for every skill subdirectory. No public API change.The production scenarios where this matters are bounded:
HERMES_HOMEis constant from start to exit — no behavioural change.HERMES_HOMEper profile across the process lifetime — same._find_all_skills()would now match the fresh-subprocess count on the next call.Type: Bug fix
Closes: #58908