fix(skills): widen call-time skills-dir resolution to skills_sync

[Fe2-O3]

Closes #65828.

tools/skills_sync is the third module in the lineage of #40677 — f8723c478 fixed skills_tool, c6a3d412d widened the same fix to skill_manager_tool, and skills_sync has the identical import-time binding with no call-time resolver.

_profile_scope (hermes_cli/web_server.py) documents the seam and hand-retargets two modules:

tools.skills_tool and tools.skill_manager_tool bind SKILLS_DIR at import time, so the override CANNOT reach them.

skills_sync is not among them, and the dashboard console reaches it in-process: console_engine.py allowlists skills opt-in|opt-out|reset|diff|list-modified → cmd_skills → skills_hub.skills_command → lazy from tools.skills_sync import ... (:1152, :1195, :1222, :1337). Hub install/update/uninstall are unaffected — those correctly use a fresh profile-scoped subprocess (web_server.py:12501).

Reproduced

With the module imported under profile A and profile B active:

skills_sync.HERMES_HOME        : .../profileA   <- frozen at import
active home                    : .../profileB

is_bundled_skills_opt_out()    -> False   # B HAS the marker; docstring promises the ACTIVE profile
set_bundled_skills_opt_out(True) -> wrote the marker into A, not B

reset_bundled_skill() is the sharp edge: its strict-child delete guard is computed from the frozen SKILLS_DIR (skills_sync.py:744) — the check that exists so a collapsed dest can't "wipe every installed skill".

The binding is order-dependent: the first console skills command to run inside a profile scope binds correctly; every later one under a different profile does not. That's likely why it has survived — it presents as intermittent.

Fix

The established pattern from both prior commits: keep the module attributes for tests and external patchers, snapshot at import, resolve live otherwise. seed_profile_skills already works around this caching with a subprocess ("Uses subprocess because sync_skills() caches HERMES_HOME at module level") — that covers seeding only, not the console.

Tests

tests/tools/test_skills_sync_profile_scope.py — behavioral, per AGENTS.md's ban on reading source in tests. Reloads the module under profile A, activates B, and asserts on real reads/writes. All 5 fail against the unfixed module; existing test_skills_sync.py + test_skills_list_modified_diff.py (75 tests) stay green.

Note on AGENTS.md:1193

"Module-level constants are fine — they cache get_hermes_home() at import time, which is AFTER _apply_profile_override() sets the env var."

True for the one-shot CLI, not for the set_hermes_home_override ContextVar in long-lived processes — which is the seam _profile_scope exists to paper over. Happy to correct it here or separately.

🤖 Generated with Claude Code

[tonydwb]

Code Review Summary

Verdict: Approved

Looks Good

• Small, focused fix: widens call-time skills-dir resolution to include skills_sync
• Two file changes (skills directory resolution + test) — appropriate scope
• No security or code quality concerns

---

Reviewed by Hermes Agent

[tonydwb]

Code Review Summary

Verdict: Comment

Widens call-time skills-dir resolution to skills_sync (193 additions / 23 deletions). Prior COMMENT activity noted.

No security issues. No debug artifacts.

---

Reviewed by Hermes Agent (cron batch 2026-07-17)

[teknium1]

Thanks for the focused profile-scope fix. The premise remains valid on current main: tools/skills_sync.py:39-41 still captures profile paths at import time, while hermes_cli/web_server.py:13742-13795 applies a context-local profile override but retargets only skills_tool and skill_manager_tool. Dashboard console commands execute in that scope (hermes_cli/web_server.py:15670-15680) and reach skills_sync through hermes_cli/skills_hub.py:1152, :1195, :1222, :1270, :1337, and :1365.

The resolver approach matches the established fixes in f8723c478 and c6a3d412d, preserves explicit module-attribute patches for existing tests, and replaces every current path use in skills_sync covered by the PR diff. The PR base is an ancestor of this checkout's HEAD; current changes after that base did not modify tools/skills_sync.py, so salvage should be mechanical.

Automated hermes-sweeper review.

[GottZ]

This was generated by AI during triage.

Summary

One PR addresses Issue #65828. #66094 replaces import-time profile-path use in tools/skills_sync.py with call-time resolvers and adds regression coverage for active-profile reads, writes, manifests, resolver restoration, and explicit monkeypatch precedence.

Related pull requests

• fix(skills): widen call-time skills-dir resolution to skills_sync #66094 best fix — (+193/-23) — n/a: The complete diff routes current HERMES_HOME, SKILLS_DIR, and MANIFEST_FILE uses through call-time resolvers, including the reset deletion guard, so dashboard console operations follow the active profile rather than the profile captured at import. The added tests reproduce two-profile operation and cover profile-local marker reads/writes, manifest lookup, resolver restoration, and legacy monkeypatch behavior; this supports the maintainer-bot keep_open review and its high-salvageability assessment.

Suggested consolidation

Keep #66094 open with a salvage path: retain the call-time resolver conversion and the focused two-profile regression suite, rebasing mechanically onto current main if needed. It is the sole PR in this complex, so there are no duplicate PRs to close.

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
    I65828(["issue #65828 (open)"])
    P66094["PR #66094 (open)"]
    P66094 -->|best fix| I65828
    class I65828 open
    class P66094 open
    class P66094 best
    class P66094 target
    click I65828 "https://github.com/NousResearch/hermes-agent/issues/65828"
    click P66094 "https://github.com/NousResearch/hermes-agent/pull/66094"

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 1 pull request and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 16 kB of PR diffs, 8 kB of issue/PR text, <1 kB of discussion (2 comments), 2 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.