Skip to content

fix(skills): resolve skills-shared from the top-level root, not a profile HERMES_HOME - #162

Merged
Kyzcreig merged 1 commit into
mainfrom
fix/shared-root-profile-home
Jul 2, 2026
Merged

Kyzcreig merged 1 commit into
mainfrom
fix/shared-root-profile-home

Conversation

@Kyzcreig

@Kyzcreig Kyzcreig commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

Symptom

Every skill_manage(create) from a non-default profile hard-errors:

SkillCreateError: category 'general' is not a shared group. Pick one of: (none found under skills-shared/)

Only the default profile can author to shared. Ground-truthed live on the fleet (athena/daedalus gateways, 2026-07-01).

Root cause

#122's _shared_skills_root() resolves HERMES_HOME / 'skills-shared'. A specialist gateway runs with HERMES_HOME=<root>/profiles/<name>, so it resolves profiles/<name>/skills-shared — which doesn't exist (the shared tree is top-level only). _valid_shared_groups() → [] → every group invalid → hard error.

Fix

_shared_skills_root() derives the top-level root with the same profile rule as hermes_constants.get_default_hermes_root (parent dir named profiles → grandparent), applied to the module's HERMES_HOME so the established test seam (patching the module global) keeps working. Standard (~/.hermes/profiles/<p>) + Docker (/opt/data/profiles/<p>) layouts covered; a non-profile home resolves exactly as before (vanilla upstream byte-identical). Fail-open to the old path on any resolution error.

Tests

New TestProfileHomeSharedRoot suite (9 tests): profile-home resolution, groups found, create lands in the top-level tree (and NOT under the profile dir), Docker layout, a RED-proof documenting the old expression's miss, vanilla/fleet layouts unchanged, fail-open. 35/35 module, 222/222 skill-tool blast radius.

…file HERMES_HOME

The author-to-shared create path (#122) resolved the shared tree as
HERMES_HOME/'skills-shared'. A specialist-profile gateway runs with
HERMES_HOME=<root>/profiles/<name>, so it resolved profiles/<name>/skills-shared
— which doesn't exist (the shared tree is top-level only). Result:
_valid_shared_groups() returned [] and EVERY create from a non-default profile
hard-errored ('category X is not a shared group … (none found)'). Only the
default profile worked.

Fix: _shared_skills_root() derives the top-level root with the same profile
rule as hermes_constants.get_default_hermes_root (parent dir named 'profiles'
-> grandparent), applied to the module's HERMES_HOME so the established test
seam (patching the module global) keeps working. Covers standard
(~/.hermes/profiles/<p>) and Docker (/opt/data/profiles/<p>) layouts; a
non-profile home resolves exactly as before (vanilla upstream unchanged).
Fail-open on any resolution error to the old HERMES_HOME-relative path.

Tests: new TestProfileHomeSharedRoot suite — profile-home resolution, groups
found, create lands in the top-level tree (and NOT under the profile), Docker
layout, a RED-proof documenting the old expression's miss, vanilla/fleet
layouts unchanged (INV-1/2), and fail-open. 35/35 module, 222/222 blast radius.
@greptile-apps

greptile-apps Bot commented Jul 2, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes a regression where skill_manage(create) hard-errored for every non-default profile because _shared_skills_root() resolved HERMES_HOME / "skills-shared" — a profile-relative path that doesn't exist when HERMES_HOME=<root>/profiles/<name>.

  • Core fix (tools/skill_manager_tool.py): _shared_skills_root() now detects profile-mode layouts by checking home.parent.name == \"profiles\" and returns home.parent.parent / \"skills-shared\" (the top-level root), exactly mirroring the get_default_hermes_root profile rule already in hermes_constants.py. Non-profile homes behave byte-for-byte as before; a broad except Exception guard provides fail-open behavior.
  • Tests (tests/tools/test_skill_manager_create_shared.py): adds a _profile_home fixture and 9 focused regression tests, including a red-proof documenting the old expression's miss, Docker layout, and fail-open behavior.

Confidence Score: 4/5

Safe to merge — the change is a small, targeted fix to a single function with no side effects on non-profile deployments.

The fix is minimal and well-contained: one function in skill_manager_tool.py gains a two-branch conditional and a fail-open guard, and all three resolution paths (profile, vanilla, error) are directly exercised by the new test suite. The logic is a faithful copy of the already-proven get_default_hermes_root profile rule, so the reasoning is easy to audit. No issues found that would block merging.

No files require special attention. Both changed files are clean.

Important Files Changed

Filename Overview
tools/skill_manager_tool.py Replaces the one-liner _shared_skills_root() with a profile-aware version that mirrors hermes_constants.get_default_hermes_root; non-profile behavior is unchanged and a fail-open guard prevents crashes on pathological paths.
tests/tools/test_skill_manager_create_shared.py Adds _profile_home fixture and TestProfileHomeSharedRoot with 9 tests covering standard profile, Docker, fail-open, red-proof of the old bug, and invariants for non-profile homes.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[_shared_skills_root called] --> B{try block}
    B --> C[home = HERMES_HOME]
    C --> D{home.parent.name == 'profiles'?}
    D -- Yes: profile layout --> E[return home.parent.parent / 'skills-shared']
    D -- No: vanilla/custom root --> F[return home / 'skills-shared']
    B -- Exception --> G[return HERMES_HOME / 'skills-shared' — fail-open fallback]
    E --> H[_valid_shared_groups iterates result]
    F --> H
    G --> H
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[_shared_skills_root called] --> B{try block}
    B --> C[home = HERMES_HOME]
    C --> D{home.parent.name == 'profiles'?}
    D -- Yes: profile layout --> E[return home.parent.parent / 'skills-shared']
    D -- No: vanilla/custom root --> F[return home / 'skills-shared']
    B -- Exception --> G[return HERMES_HOME / 'skills-shared' — fail-open fallback]
    E --> H[_valid_shared_groups iterates result]
    F --> H
    G --> H
Loading

Reviews (1): Last reviewed commit: "fix(skills): resolve skills-shared from ..." | Re-trigger Greptile

@Kyzcreig
Kyzcreig merged commit 7bf9e2b into main Jul 2, 2026
33 checks passed
@Kyzcreig
Kyzcreig deleted the fix/shared-root-profile-home branch July 2, 2026 02:36
Kyzcreig pushed a commit that referenced this pull request Jul 31, 2026
… 202 handling

Client leg of M2 org-shared skills (hsp-1-contract.md §11), pairing with
gateway-gateway #162 and NAS #768 (both merged).

- resolve_org_identity(): org_id + org_role from the token claims. NO
  org_role claim (personal org — NAS only stamps it for multi-member orgs)
  => SyncInertError => every org surface is inert; personal M1 sync
  untouched (contract §11.1 REFINED). org_sync_available() for callers.
- pull_org_skills(): materialize the org canonical set (refs/org/<org_id>/
  HEAD) into ~/.hermes/skills/_org/<org_id>/ — fast-forward only, no client
  merge on the org path (design.md §2.6); read-only mirror by convention
  (§7.1: a local edit is a personal fork until proposed). maybe_pull_org_
  skills() best-effort hook (never raises; inert without the claim).
- propose_skill(): snapshot the LOCAL skill dir, splice it into the org
  HEAD's skill-tree map (per-skill delta, never wholesale replace), upload
  ?scope=org, CAS the org HEAD. ADMIN => direct merge; MEMBER => server
  converts to a proposal — cas_ref now surfaces 202 as
  {proposal_pending: True, proposal_id, ref} (success-shaped, NEVER
  presented as live). Non-interactive by design for the future automated
  submitter (Ben's trajectory note).
- put_objects(org_scope=True) adds ?scope=org (contract §11.5).
- is_sync_eligible(): skills under _org/ are excluded from PERSONAL sync —
  enterprise content never rides a personal push (§11.11).
- CLI: hermes skills propose <name> [-m msg] — prints 'pending admin
  review' for 202, 'merged' for admin, and a plain 'org sync unavailable'
  for personal orgs instead of a raw 403.

Tests: 56 in the sync client suite (+10 org: identity gate both ways, _org/
personal-sync exclusion, admin direct merge, member 202 w/ HEAD untouched +
proposal ref parked + never-merged, splice-not-replace root, pull mirror
materialization, no-head noop, org-feature gate, maybe_pull inert). Mock
server extended (org feature flag, member-CAS→202). 103 across skills
suites. Live CLI smoke: propose --help + personal-org inert path verified.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant