feat(skills): skills.shared shorthand for cross-profile sharing

[cyb0rgk1tty]

What does this PR do?

Adds a small, opt-in convention for sharing specific skills across some (but not all) profiles on a single host, without requiring users to repeat full directory paths in every profile's config.yaml.

The convention:

~/.hermes/shared-skills/
├── team-runbook/
│   └── SKILL.md
└── company-style-guide/
    └── SKILL.md

Profiles opt in by name in their config.yaml:

skills:
  shared:
    - team-runbook
    - company-style-guide

Internally, skills.shared resolves to subdirectories of ~/.hermes/shared-skills/ that are appended to the skill search path between the profile-local skills/ dir and any skills.external_dirs entries. The end result is identical to listing the individual skill directories under skills.external_dirs (which has always worked) — this just gives a shorthand and a canonical location.

Why a shorthand instead of asking users to use external_dirs?

• Less typing. List a name once instead of a full path per profile.
• Less error-prone. Bare names with strict validation (no slashes, no .. traversal, no leading dot) are harder to mis-configure than raw paths.
• Discoverability. The docs can point at one canonical location.
• Composes cleanly with copy-on-write for shared skills (see fix(skills): copy-on-write for agent edits of external skills #5407, the prerequisite PR) — the agent never mutates the shared source, only profile-local overrides.

What it is NOT

• Not automatic. Behavior is fully opt-in: profiles with no skills.shared key see no shared skills. There is no implicit inclusion of ~/.hermes/shared-skills/ contents — sharing is always explicit, per-profile, per-skill. This preserves the existing profile-isolation guarantee.
• Not a replacement for external_dirs. skills.external_dirs is still the right tool for skills outside ~/.hermes/, env-var-substituted paths, or whole directories of skills.
• Not a copy. The skill lives in exactly one place on disk (~/.hermes/shared-skills/<name>/); profiles reference it. Edits in one profile don't affect another (copy-on-write handles this — see fix(skills): copy-on-write for agent edits of external skills #5407).

Related Issue / PR

Depends on #5407 (fix(skills): copy-on-write for agent edits of external skills). That PR ensures that when an agent edits a shared skill, the edit becomes a profile-local override instead of mutating the shared source. Without it, this PR would be a footgun.

This PR is mergeable on its own, but the safety story is only complete with both.

Type of Change

• ✨ New feature (non-breaking change that adds functionality)

Changes Made

agent/skill_utils.py

• New constant DEFAULT_SHARED_SKILLS = ~/.hermes/shared-skills/.
• New function get_shared_skill_dirs(): reads skills.shared from the active profile's config.yaml, validates each entry (rejects path traversal, slashes, hidden directories), returns paths to existing subdirectories under DEFAULT_SHARED_SKILLS. Invalid names and missing directories are silently skipped so a typo or half-provisioned host doesn't break the agent.
• Updated get_all_skills_dirs(): inserts shared skill dirs between the profile-local skills/ dir and skills.external_dirs entries. Duplicates (same resolved path) are filtered, so listing a shared skill in both skills.shared and skills.external_dirs only includes it once. Updated docstring documents the full precedence order.

tests/agent/test_external_skills.py — 10 new tests

• TestGetSharedSkillDirs:
  • test_no_shared_config_returns_empty — no skills.shared key → empty list
  • test_shared_skill_resolved_by_name — name is resolved to the right directory
  • test_missing_shared_skill_silently_skipped — typo in config doesn't break discovery
  • test_path_traversal_rejected — ../../etc, foo/bar, .hidden are dropped
  • test_string_value_converted_to_list — shared: name (single string) works
• TestSharedInGetAllSkillsDirs:
  • test_shared_appears_after_local_before_external — order is local → shared → external
  • test_no_shared_config_backward_compat — pre-PR behavior preserved when key absent
  • test_shared_not_duplicated_when_also_in_external_dirs — dedup works
• TestSelectiveSharingAcrossProfiles:
  • test_profile_a_and_b_see_different_subsets — proves the per-profile, per-skill selectivity
• TestLocalShadowsShared:
  • test_profile_local_skill_takes_precedence_over_shared_by_position — local skills come first

website/docs/user-guide/features/skills.md

• New section "Sharing Specific Skills Across Profiles" documenting the convention, config syntax, relationship to external_dirs, precedence rules, and a pointer to the read-only / copy-on-write semantics from the External Skill Directories section.

How to Test

# 1. Run the new tests
pytest tests/agent/test_external_skills.py::TestGetSharedSkillDirs \
       tests/agent/test_external_skills.py::TestSharedInGetAllSkillsDirs \
       tests/agent/test_external_skills.py::TestSelectiveSharingAcrossProfiles \
       tests/agent/test_external_skills.py::TestLocalShadowsShared -v
# Expect: 10 passed

# Full external_skills suite (regression check)
pytest tests/agent/test_external_skills.py -q
# Expect: 21 passed

# 2. Manual end-to-end verification
mkdir -p ~/.hermes/shared-skills/team-runbook
cat > ~/.hermes/shared-skills/team-runbook/SKILL.md <<MD
---
name: team-runbook
description: Shared team operational runbook.
---

# Team Runbook

Shared step here.
MD

# Add to a profile's config:
#   skills:
#     shared:
#       - team-runbook
# Then start the agent and run: /skills_list
# Confirm "team-runbook" appears.

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (feat(skills): ...)
• I searched for existing PRs to make sure this isn't a duplicate
• My PR contains only changes related to this feature
• All new tests pass; full tests/agent/test_external_skills.py suite passes
• I've added tests for my changes
• I've tested on my platform: Ubuntu 24.04 / Python 3.12

Documentation & Housekeeping

• I've updated relevant documentation (website/docs/user-guide/features/skills.md)
• N/A — no changes to cli-config.yaml.example (the new key is per-profile, not global)
• N/A — no architecture changes
• N/A — Python-only change, no platform-specific code paths
• N/A — no tool behavior changes (skill_manage tool already handles external/shared correctly via fix(skills): copy-on-write for agent edits of external skills #5407)

[WompaJango]

Related design point now tracked in #19451. I like this PR as a selective shared-skills mechanism, but I think there is still a broader semantics issue: users naturally interpret "global skills" as automatically global across profiles. #19451 proposes making that a first-class layer with precedence profile-local -> global -> external_dirs, so the word "global" actually means global.

[teknium1]

Thanks for the focused selective-sharing proposal. skills.shared is not present on current main, so the shorthand itself is not superseded.

Problems

• The bundled copy-on-write implementation changes all skills.external_dirs, not only the new shared convention. Main deliberately supports writable external skills being updated in place: commit 8c8fc6c1 established that behavior, its regression coverage is at tests/tools/test_skill_manager_tool.py:760-864, and the current contract is documented at website/docs/user-guide/features/skills.md:325-326.
• Adding shared roots only through get_all_skills_dirs() (PR agent/skill_utils.py:305-310) would not integrate them with current external-ownership checks. Main's curator guard uses is_external_skill_path() (tools/skill_manager_tool.py:337-349), which only checks get_external_skills_dirs() (agent/skill_utils.py:579-596). Shared skills therefore need an explicit lifecycle/ownership treatment.

Suggested changes

• Split the shorthand from the generic external-directory copy-on-write reversal, then align shared-root ownership with curator protections and add coverage.
• Preserve the current mtime-keyed config-cache approach when implementing shared-name resolution.

Automated hermes-sweeper review.

[teknium1]

This classification applies copy-on-write to every skills.external_dirs skill, not just the proposed shared root. Current main intentionally supports in-place external mutation (commit 8c8fc6c1, with regression coverage in TestExternalSkillMutations); please separate the shared-layer policy from that established compatibility behavior.

[teknium1]

Appending this root to discovery alone leaves it outside current external-ownership checks: is_external_skill_path() iterates only get_external_skills_dirs(), and curator guards depend on that helper. Register shared roots with the ownership/lifecycle path too, and add a curator/background-write regression test.

[teknium1]

Please use the existing mtime-keyed config cache when salvaging this. get_all_skills_dirs() is called repeatedly during skill discovery; reparsing config.yaml here on every call recreates the startup-cost issue current main avoids for external_dirs.

[WompaJango]

Correction posted below — I originally wrote this comment against current main HEAD instead of the PR's base SHA (6f1cb46df, April 6). Several claims were wrong as a result. Please read the correction.

---

Thanks for this — the skills.shared shorthand itself is a clean, well-reasoned feature (name-based opt-in, traversal validation, dedup, backward compat). I went through the diff and want to share what I found.

⚠️ The PR needs a rebase — main has fundamentally changed since this base

This PR is built on 6f1cb46df (April 6). Since then, main has had major changes to the external-skills subsystem:

• 8c8fc6c1e (fix(skills): let skill_manage modify skills in external_dirs in place #17512, April 29): reversed external skills from read-only to in-place editable. Docs changed from "Read-only: External dirs are only scanned for skill discovery" to "Create locally, update in place".
• 96bc524a7 (June 24): added is_external_skill_path() and curator ownership guards protecting external skills from background curation.
• 4e6d05c6a (perf(skills): share raw config cache in skill utils #46149, June 14): added the mtime-keyed _EXTERNAL_DIRS_CACHE to get_external_skills_dirs().

Against the PR's own base, the CoW changes are consistent — the base docs say read-only. But after rebasing to current main, there will be real conflicts:

Conflict 1: CoW vs. in-place editing

The CoW changes in tools/skill_manager_tool.py would conflict with main's TestExternalSkillMutations (added in 8c8fc6c1e), which explicitly assert in-place writes and assert not (local / "ext-skill").exists(). Those tests would fail with the CoW code.

Decision needed: Should CoW apply to shared skills only, or reverse main's in-place contract for all external skills? If the former, scope the CoW to get_shared_skill_dirs() roots only. If the latter, that's a separate breaking-change PR with its own docs and test updates.

Conflict 2: Shared skills outside ownership guard

After rebase, is_external_skill_path() (added in 96bc524a7) checks only get_external_skills_dirs(). Shared skill dirs added via get_shared_skill_dirs() would be invisible to the curator guard. The curator could mutate/delete shared skills — defeating the read-only guarantee.

Fix: Make is_external_skill_path() also check get_shared_skill_dirs().

Conflict 3: No config cache

After rebase, get_external_skills_dirs() has an mtime-keyed cache (4e6d05c6a), but get_shared_skill_dirs() would parse YAML on every call. Since get_all_skills_dirs() is called once per skill during discovery, this recreates the startup cost that the cache was added to fix.

Fix: Mirror the existing cache pattern in get_shared_skill_dirs().

Valid regardless of base: DEFAULT_SHARED_SKILLS at import time

DEFAULT_SHARED_SKILLS = Path.home() / ".hermes" / "shared-skills"

Path.home() is called once at module load. The tests work around this with importlib.reload(su) in every test case. Computing it lazily inside the function (or via get_hermes_home()) would eliminate the reload hacks.

Summary

The skills.shared shorthand is genuinely useful. The main work item is a rebase + resolving the three conflicts above. The CoW question (shared-only vs. universal) is the key design decision.