diff --git a/agent/prompt_builder.py b/agent/prompt_builder.py index 2b1f3cfc5be39..f4766141a39f3 100644 --- a/agent/prompt_builder.py +++ b/agent/prompt_builder.py @@ -19,13 +19,18 @@ from typing import Optional from agent.runtime_cwd import resolve_agent_cwd from agent.skill_utils import ( EXCLUDED_SKILL_DIRS, + ORG_ACTIVE_MARKER, + ORG_MIRROR_DIR_NAME, + ORG_PROVENANCE_FILE, SKILL_SUPPORT_DIRS, extract_skill_conditions, extract_skill_description, get_all_skills_dirs, get_disabled_skill_names, iter_skill_index_files, + org_id_of_path, parse_frontmatter, + read_active_org_id, skill_matches_environment, skill_matches_platform, skill_matches_platform_list, @@ -1340,7 +1345,9 @@ def drain_truncation_warnings() -> list: _SKILLS_PROMPT_CACHE_MAX = 8 _SKILLS_PROMPT_CACHE: OrderedDict[tuple, str] = OrderedDict() _SKILLS_PROMPT_CACHE_LOCK = threading.Lock() -_SKILLS_SNAPSHOT_VERSION = 1 +# v2: entries gained org provenance fields (org_id/org_author/rel_dir) for M2 +# org-shared skills; older snapshots are discarded and rebuilt. +_SKILLS_SNAPSHOT_VERSION = 2 def _skills_prompt_snapshot_path() -> Path: @@ -1359,13 +1366,32 @@ def clear_skills_system_prompt_cache(*, clear_snapshot: bool = False) -> None: def _build_skills_manifest(skills_dir: Path) -> dict[str, list[int]]: - """Build an mtime/size manifest of all SKILL.md and DESCRIPTION.md files.""" + """Build an mtime/size manifest of all SKILL.md and DESCRIPTION.md files. + + Org mirrors (M2): only the ACTIVE org's mirror participates, and the + ``.active_org`` marker itself is included — so switching/leaving an org + invalidates the snapshot even when no SKILL.md changed. + """ manifest: dict[str, list[int]] = {} skills_dir_str = str(skills_dir) base = os.path.join(skills_dir_str, "") prefix_len = len(base) + active_org = read_active_org_id(skills_dir) + org_root = os.path.join(skills_dir_str, ORG_MIRROR_DIR_NAME) + marker_path = os.path.join(org_root, ORG_ACTIVE_MARKER) + try: + st = os.stat(marker_path) + manifest[ORG_MIRROR_DIR_NAME + "/" + ORG_ACTIVE_MARKER] = [ + int(st.st_mtime), int(st.st_size), + ] + except OSError: + pass for root, dirs, files in os.walk(skills_dir_str, followlinks=True): has_skill_md = "SKILL.md" in files + if root == skills_dir_str and ORG_MIRROR_DIR_NAME in dirs and active_org is None: + dirs.remove(ORG_MIRROR_DIR_NAME) + elif root == org_root: + dirs[:] = [d for d in dirs if d == active_org] dirs[:] = [ d for d in dirs @@ -1430,6 +1456,15 @@ def _build_snapshot_entry( """Build a serialisable metadata dict for one skill.""" rel_path = skill_file.relative_to(skills_dir) parts = rel_path.parts + + # M2 org mirror: strip the `_org//` prefix so category/name derive + # from the path WITHIN the mirror (same shape the org tree was built + # from), and record provenance for labeling + fail-loud collisions. + org_id: str | None = None + if len(parts) >= 3 and parts[0] == ORG_MIRROR_DIR_NAME: + org_id = parts[1] + parts = parts[2:] + if len(parts) >= 2: skill_name = parts[-2] category = "/".join(parts[:-2]) if len(parts) > 2 else parts[0] @@ -1441,7 +1476,7 @@ def _build_snapshot_entry( if isinstance(platforms, str): platforms = [platforms] - return { + entry = { "skill_name": skill_name, "category": category, "frontmatter_name": str(frontmatter.get("name", skill_name)), @@ -1449,6 +1484,22 @@ def _build_snapshot_entry( "platforms": [str(p).strip() for p in platforms if str(p).strip()], "conditions": extract_skill_conditions(frontmatter), } + if org_id: + entry["org_id"] = org_id + # Author from the pull-time provenance sidecar (token-verified at + # push by the plane's author_mismatch guard). Best-effort. + try: + import json as _json + + prov_path = ( + skills_dir / ORG_MIRROR_DIR_NAME / org_id / ORG_PROVENANCE_FILE + ) + prov = _json.loads(prov_path.read_text(encoding="utf-8")) + device = str(prov.get("author_device") or "") + entry["org_author"] = device or str(prov.get("author_user_id") or "") + except Exception: + entry["org_author"] = "" + return entry # ========================================================================= @@ -1584,6 +1635,10 @@ def build_skills_system_prompt( skills_by_category: dict[str, list[tuple[str, str]]] = {} category_descriptions: dict[str, str] = {} + # Unified visible-entry list (both paths) so the org labeling + + # fail-loud collision pass below runs identically for snapshot and scan. + visible_entries: list[dict] = [] + skill_entries: list[dict] = [] if snapshot is not None: # Fast path: use pre-parsed metadata from disk @@ -1591,7 +1646,6 @@ def build_skills_system_prompt( if not isinstance(entry, dict): continue skill_name = entry.get("skill_name") or "" - category = entry.get("category") or "general" frontmatter_name = entry.get("frontmatter_name") or skill_name platforms = entry.get("platforms") or [] if not skill_matches_platform_list(platforms): @@ -1604,16 +1658,13 @@ def build_skills_system_prompt( available_toolsets, ): continue - skills_by_category.setdefault(category, []).append( - (frontmatter_name, entry.get("description", "")) - ) + visible_entries.append(entry) category_descriptions = { str(k): str(v) for k, v in (snapshot.get("category_descriptions") or {}).items() } else: # Cold path: full filesystem scan + write snapshot for next time - skill_entries: list[dict] = [] for skill_file in iter_skill_index_files(skills_dir, "SKILL.md"): is_compatible, frontmatter, desc = _parse_skill_file(skill_file) entry = _build_snapshot_entry(skill_file, skills_dir, frontmatter, desc) @@ -1629,10 +1680,38 @@ def build_skills_system_prompt( available_toolsets, ): continue - skills_by_category.setdefault(entry["category"], []).append( - (entry["frontmatter_name"], entry["description"]) - ) + visible_entries.append(entry) + # ── M2 org labeling + FAIL-LOUD collisions ───────────────────────── + # An org skill lists with an explicit provenance tag. When a personal and + # an org skill share a name, NEITHER silently wins: both list qualified + # (personal keeps the bare name is the wrong default — silent divergence + # from the org set; org winning silently shadows the user's own work) — + # so both entries carry a [name collision] flag and skill_view refuses + # the ambiguous bare name (its existing multi-candidate guard). + name_owners: dict[str, set[str]] = {} + for entry in visible_entries: + fm = entry.get("frontmatter_name") or entry.get("skill_name") or "" + kind = "org" if entry.get("org_id") else "personal" + name_owners.setdefault(fm, set()).add(kind) + for entry in visible_entries: + fm = entry.get("frontmatter_name") or entry.get("skill_name") or "" + desc = entry.get("description", "") + org_id = entry.get("org_id") + collided = len(name_owners.get(fm, set())) > 1 + if org_id: + author = entry.get("org_author") or "" + tag = f"[org-shared{': by ' + author if author else ''}]" + desc = f"{tag} {desc}".strip() + category = f"org:{org_id}" + else: + category = entry.get("category") or "general" + if collided: + desc = f"[name collision — also exists {'personally' if org_id else 'in your org'}; load via category path] {desc}".strip() + skills_by_category.setdefault(category, []).append((fm, desc)) + + if snapshot is None: + # (continuation of the cold path below: category descriptions + write) # Read category-level DESCRIPTION.md files for desc_file in iter_skill_index_files(skills_dir, "DESCRIPTION.md"): try: diff --git a/agent/skill_utils.py b/agent/skill_utils.py index eea78d6a07c05..a302c6981a478 100644 --- a/agent/skill_utils.py +++ b/agent/skill_utils.py @@ -49,6 +49,55 @@ EXCLUDED_SKILL_DIRS = frozenset( # archive workflow preserves a complete old skill package under references/. SKILL_SUPPORT_DIRS = frozenset(("references", "templates", "assets", "scripts")) +# ── Org-shared skills (sync contract) ─────────────────────────── +# Org mirrors live under ~/.hermes/skills/_org//. Resolution is +# TOKEN-GATED via a marker file the sync client writes after verifying the +# token (skills_sync_client.pull_org_skills): only the marked org's mirror is +# scanned. No marker ⇒ no org skills load. The marker is plain data (org_id +# string) so this module stays import-light; the VERIFICATION lives in the +# sync client, which is the only writer. Offline grace: the marker persists, +# so already-pulled org skills keep working without connectivity; a VERIFIED +# org change (or personal-org token) rewrites/removes it. + +ORG_MIRROR_DIR_NAME = "_org" +ORG_ACTIVE_MARKER = ".active_org" +ORG_PROVENANCE_FILE = ".org-provenance.json" +# Records the fingerprint of each skill exactly as upstream sent it, so a +# later local edit is detectable and an org pull can refuse to clobber it. +ORG_BASELINE_FILE = ".org-baseline.json" + + +def read_active_org_id(skills_dir: Path) -> Optional[str]: + """The org id whose mirror may resolve, or None (no org skills load).""" + try: + marker = skills_dir / ORG_MIRROR_DIR_NAME / ORG_ACTIVE_MARKER + if not marker.exists(): + return None + val = marker.read_text(encoding="utf-8").strip() + return val or None + except OSError: + return None + + +def is_org_mirror_path(path, skills_dir: Path) -> bool: + """True when *path* is inside the org mirror (``_org/``).""" + try: + rel = Path(path).resolve().relative_to(Path(skills_dir).resolve()) + except (OSError, ValueError): + return False + return bool(rel.parts) and rel.parts[0] == ORG_MIRROR_DIR_NAME + + +def org_id_of_path(path, skills_dir: Path) -> Optional[str]: + """The ```` segment for a path under ``_org//...``.""" + try: + rel = Path(path).resolve().relative_to(Path(skills_dir).resolve()) + except (OSError, ValueError): + return None + if len(rel.parts) >= 2 and rel.parts[0] == ORG_MIRROR_DIR_NAME: + return rel.parts[1] + return None + def is_excluded_skill_path(path, *, root: Optional[Path] = None) -> bool: """True if *path* should be skipped by active skill scanners. @@ -817,11 +866,24 @@ def iter_skill_index_files(skills_dir: Path, filename: str): scripts) can contain arbitrary markdown and even archived package ``SKILL.md`` files, but they are progressive-disclosure data loaded through ``skill_view(..., file_path=...)`` rather than active skill roots. + + M2 org mirrors (``_org/``): TOKEN-GATED resolution. Only the active org's + subdir (per the sync-client-written ``.active_org`` marker) is walked; + every other ``_org//`` (stale mirror from a previous org, or no + marker at all) is pruned — leave an org and its skills stop resolving, + without any manual cleanup. """ skills_dir_str = str(skills_dir) + active_org = read_active_org_id(skills_dir) + org_root = os.path.join(skills_dir_str, ORG_MIRROR_DIR_NAME) matches: list[str] = [] for root, dirs, files in os.walk(skills_dir_str, followlinks=True): has_skill_md = "SKILL.md" in files + if root == skills_dir_str and ORG_MIRROR_DIR_NAME in dirs and active_org is None: + dirs.remove(ORG_MIRROR_DIR_NAME) + elif root == org_root: + # Inside _org/: descend ONLY into the active org's mirror. + dirs[:] = [d for d in dirs if d == active_org] dirs[:] = [ d for d in dirs diff --git a/cli.py b/cli.py index b0034f4da3c92..1094496edd22d 100644 --- a/cli.py +++ b/cli.py @@ -14650,6 +14650,26 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin, CLIBillingMixin): ) except Exception: pass + + # Skill sync — best-effort periodic pull, piggy-backing on the + # curator tick. Inert unless the access gate is open and a sync base + # URL is configured; swallows all errors so it never blocks startup. + try: + from tools.skills_sync_client import maybe_pull_skills + maybe_pull_skills() + except Exception: + pass + + # Org-shared skills — pull the organisation's approved set into the + # read-only mirror. Gated on real org membership: resolve_org_identity + # requires an org role on the token, which is only issued for + # multi-member organisations, so a solo account never reaches the + # network here. Fail-quiet, exactly like the personal pull above. + try: + from tools.skills_sync_client import maybe_pull_org_skills + maybe_pull_org_skills() + except Exception: + pass if self.preloaded_skills and not self._startup_skills_line_shown: skills_label = ", ".join(self.preloaded_skills) self._console_print( diff --git a/gateway/run.py b/gateway/run.py index 9c39dcb7bc171..4097139688d95 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -24718,6 +24718,23 @@ def _start_gateway_housekeeping(stop_event: threading.Event, adapters=None, loop except Exception as e: logger.debug("Curator tick error: %s", e) + # Skill Sync — best-effort periodic pull on the same cadence. + # Inert unless the access gate is open and a sync base URL is + # configured; never raises. + try: + from tools.skills_sync_client import maybe_pull_skills + maybe_pull_skills() + except Exception as e: + logger.debug("Sync pull tick error: %s", e) + + # Org-shared skills. Gated on real org membership (the token must + # carry an org role), so a solo account never reaches the network. + try: + from tools.skills_sync_client import maybe_pull_org_skills + maybe_pull_org_skills() + except Exception as e: + logger.debug("Org sync pull tick error: %s", e) + # Stale-session auto-archive — a live timer, so gateways that stay up # for weeks keep sweeping on schedule (the startup hook fires once). # maybe_auto_archive() is gated by sessions.min_interval_hours in diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 4db17c3e872d8..e7859bd351bd5 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -433,6 +433,7 @@ import functools as _functools from hermes_cli.sessions_cmd import cmd_sessions # noqa: F401 from hermes_cli.subcommands._shared import add_accept_hooks_flag as _add_accept_hooks_flag from hermes_cli.subcommands.cron import build_cron_parser +from hermes_cli.subcommands.sync import build_sync_parser from hermes_cli.subcommands.gateway import build_gateway_parser from hermes_cli.subcommands.profile import build_profile_parser from hermes_cli.subcommands.model import build_model_parser @@ -4536,6 +4537,201 @@ def cmd_cron(args): cron_command(args) +def cmd_sync(args): + """Skill Sync — personal sync across devices, plus sharing with your org.""" + import json as _json + + sub = getattr(args, "sync_command", None) + + if sub in {None, ""}: + print( + "usage: hermes sync " + "\n" + "\n" + "Your skills, across your devices:\n" + " status Show what is synced, and from where\n" + " pull Pull your synced skills\n" + " push Push your opted-in skills\n" + " now Reconcile now: pull then push\n" + " enable Include a skill in your sync\n" + " disable Exclude a skill from your sync\n" + " device [--name N] Show or set this device's label\n" + "\n" + "Shared with your team:\n" + " propose Share a skill with your organisation", + file=sys.stderr, + ) + return 1 + + if sub == "device": + from tools import skills_sync_client as ssc + + name = getattr(args, "device_name", None) + if name is not None: + try: + stored = ssc.set_device_name(name) + except ValueError as e: + print(f"error: {e}", file=sys.stderr) + return 1 + print(f"device label set to '{stored}'.") + print( + "New commits from this device will use this label; existing " + "commits keep their previous one.", + file=sys.stderr, + ) + return 0 + # No --name: print the current (creating a default on first use). + print(ssc.stable_device_id()) + return 0 + + if sub == "propose": + from tools import skills_sync_client as ssc + + name = args.name + try: + result = ssc.propose_skill(name, message=args.message) + except ssc.SyncInertError as e: + print(f"cannot share this skill: {e}", file=sys.stderr) + return 1 + except ssc.SyncError as e: + print(f"could not share '{name}': {e}", file=sys.stderr) + return 1 + if result.get("proposal_pending"): + print( + f"Shared '{name}' with your organisation — an admin needs to " + f"approve it (proposal #{result.get('proposal_id')}). It is " + f"not live for the team until then." + ) + else: + print(f"Added '{name}' to your organisation's shared skills.") + return 0 + + if sub in {"enable", "disable"}: + from tools.skill_usage import set_sync, is_curation_eligible + + skill = args.skill + if not is_curation_eligible(skill): + print( + f"'{skill}' is not sync-eligible (bundled, hub-installed, " + f"external, or not found). Only agent-created / user-authored " + f"skills under ~/.hermes/skills/ can sync.", + file=sys.stderr, + ) + return 1 + set_sync(skill, sub == "enable") + print(f"sync {'enabled' if sub == 'enable' else 'disabled'} for '{skill}'.") + return 0 + + from tools import skills_sync_client as ssc + + if sub == "status": + status = ssc.sync_status() + print(_json.dumps(status, indent=2, ensure_ascii=False)) + if status.get("org_available"): + n = len(status.get("org_skills") or []) + modified = status.get("org_skills_modified") or [] + print( + f"\nOrg skills: {n} shared skill(s) from your organisation " + f"(your role: {status.get('org_role')}). They load alongside " + f"your own, labeled by origin, and you can edit them.", + file=sys.stderr, + ) + if modified: + print( + f" {len(modified)} with local edits not yet shared: " + f"{', '.join(modified)}\n" + f" Share them back with `hermes sync propose `. " + f"Org updates will not overwrite them.", + file=sys.stderr, + ) + elif status.get("logged_in"): + print( + "\nOrg skills: not applicable — this account isn't a member " + "of a shared organisation.", + file=sys.stderr, + ) + if not status.get("logged_in"): + print("\nNot logged into Nous Portal — sync is inert.", file=sys.stderr) + elif not status.get("nous_admin"): + print( + "\nSync is not enabled for your account yet.", + file=sys.stderr, + ) + elif not status.get("feature_enabled"): + print( + "\nSync feature is off for this instance (set HERMES_SYNC_ENABLED=1 " + "or config.yaml sync.enabled: true). Sync is inert.", + file=sys.stderr, + ) + elif not status.get("base_url"): + print( + "\nNo sync base URL configured (config.yaml sync.base_url or " + "HERMES_SYNC_BASE_URL). Sync is inert.", + file=sys.stderr, + ) + return 0 + + # pull / push / now — enforce the gate up front with a clear message. + try: + identity = ssc.resolve_identity() + except ssc.SyncInertError as e: + print(f"sync inert: {e}", file=sys.stderr) + return 1 + if not identity.get("nous_admin"): + print( + "sync unavailable: not enabled for your account yet.", + file=sys.stderr, + ) + return 1 + if not ssc.resolve_sync_base_url(): + print( + "sync inert: no sync base URL configured (config.yaml sync.base_url " + "or HERMES_SYNC_BASE_URL).", + file=sys.stderr, + ) + return 1 + + try: + if sub == "pull": + result = ssc.pull_skills(identity=identity) + # Refresh the org mirror too when this account belongs to an + # organisation (no-op otherwise), so one pull covers both. + org_result = ssc.maybe_pull_org_skills() + if org_result: + n = len(org_result.get("updated") or []) + print( + f"org: refreshed {n} shared skill(s) from your " + f"organisation.", + file=sys.stderr, + ) + clashes = org_result.get("conflicted") or [] + if clashes: + print( + f"org: {len(clashes)} skill(s) have BOTH local edits " + f"and org updates, so they were left as-is: " + f"{', '.join(clashes)}\n" + f" Your local version is intact. Review it, then " + f"either propose it or delete the local copy and pull " + f"again to take the org version.", + file=sys.stderr, + ) + elif sub == "push": + result = ssc.push_skills(identity=identity, message="hermes sync push") + elif sub == "now": + pull_res = ssc.pull_skills(identity=identity) + push_res = ssc.push_skills(identity=identity, message="hermes sync now") + result = {"pull": pull_res, "push": push_res} + else: + print(f"Unknown sync subcommand: {sub}", file=sys.stderr) + return 1 + except ssc.SyncError as e: + print(f"sync failed: {e}", file=sys.stderr) + return 1 + + print(_json.dumps(result, indent=2, ensure_ascii=False)) + return 0 + + def cmd_webhook(args): """Webhook subscription management.""" from hermes_cli.webhook import webhook_command @@ -10232,7 +10428,7 @@ _BUILTIN_SUBCOMMANDS = frozenset( "project", "proxy", "prompt-size", "send", "sessions", "setup", - "skin", "skills", "slack", "status", "tools", "uninstall", "update", + "skin", "skills", "slack", "status", "sync", "tools", "uninstall", "update", "version", "webhook", "whatsapp", "whatsapp-cloud", "chat", "secrets", "security", # Help-ish invocations — plugin commands not being listed in # top-level --help is an acceptable trade-off for skipping an @@ -11096,6 +11292,7 @@ def main(): # cron command (parser built in hermes_cli/subcommands/cron.py) # ========================================================================= build_cron_parser(subparsers, cmd_cron=cmd_cron) + build_sync_parser(subparsers, cmd_sync=cmd_sync) # ========================================================================= # webhook command (parser built in hermes_cli/subcommands/webhook.py) diff --git a/hermes_cli/subcommands/skills.py b/hermes_cli/subcommands/skills.py index e5f4410fe32e6..4eb68a01b1c7a 100644 --- a/hermes_cli/subcommands/skills.py +++ b/hermes_cli/subcommands/skills.py @@ -312,4 +312,5 @@ def build_skills_parser(subparsers, *, cmd_skills: Callable) -> None: "config", help="Interactive skill configuration — enable/disable individual skills", ) + skills_parser.set_defaults(func=cmd_skills) diff --git a/hermes_cli/subcommands/sync.py b/hermes_cli/subcommands/sync.py new file mode 100644 index 0000000000000..48eb133407235 --- /dev/null +++ b/hermes_cli/subcommands/sync.py @@ -0,0 +1,99 @@ +"""``hermes sync`` subcommand parser — Skill Sync. + +Cloned from ``hermes_cli/subcommands/cron.py`` — same injected-handler shape +(``func=cmd_sync``) so this module does not import ``main`` (cycle avoidance). + +Skill Sync covers two surfaces, both under this one command for launch: + + Personal — your own skills, across your own devices: + hermes sync status show gate/opt-in/head state + hermes sync pull pull and materialize opted-in skills + hermes sync push push opted-in skills + hermes sync now reconcile: pull then push + hermes sync enable opt a skill into sync + hermes sync disable opt a skill out of sync + hermes sync device [--name] show or set this device's label + + Organisation — skills shared with your team: + hermes sync propose share a skill with your organisation + +Sync is INERT unless the resolved Nous token carries the access-gate claim +AND a sync base URL is configured. The commands report that state rather than +failing opaquely. +""" + +from __future__ import annotations + +import argparse +from typing import Callable + + +def build_sync_parser(subparsers, *, cmd_sync: Callable) -> None: + """Attach the ``sync`` subcommand (and its sub-actions) to ``subparsers``.""" + sync_parser = subparsers.add_parser( + "sync", + help="Skill Sync — sync your skills across devices and with your team", + description=( + "Skill Sync keeps your skills with you. Personal sync moves your " + "own skills between your devices; if you belong to an " + "organisation, you also get its shared skills and can propose " + "your own back to the team." + ), + epilog=( + "Examples:\n" + " hermes sync status what is synced, and from where\n" + " hermes sync enable my-skill include a skill in your sync\n" + " hermes sync now pull, then push\n" + " hermes sync propose my-skill share a skill with your team\n" + ), + formatter_class=argparse.RawDescriptionHelpFormatter, + ) + sync_sub = sync_parser.add_subparsers(dest="sync_command") + + sync_sub.add_parser("status", help="Show what is synced, and from where") + sync_sub.add_parser( + "pull", help="Pull your synced skills (and your organisation's)" + ) + sync_sub.add_parser("push", help="Push your opted-in skills") + sync_sub.add_parser("now", help="Reconcile now: pull then push") + + enable = sync_sub.add_parser("enable", help="Include a skill in your sync") + enable.add_argument("skill", help="Skill name (frontmatter name / directory name)") + + disable = sync_sub.add_parser("disable", help="Exclude a skill from your sync") + disable.add_argument("skill", help="Skill name (frontmatter name / directory name)") + + device = sync_sub.add_parser( + "device", + help="Show or set this device's label (shown in the sync console)", + ) + device.add_argument( + "--name", + dest="device_name", + default=None, + help="Set a human-friendly label for this device (e.g. \"Ben's Laptop\"). " + "Omit to print the current label.", + ) + + # Org-shared skills. A member's submission becomes a proposal an admin + # reviews; an admin's merges straight into the shared set. Accounts that + # aren't in a shared organisation are told so plainly. + propose = sync_sub.add_parser( + "propose", + help="Share a skill with your organisation", + description=( + "Submit one of your skills to your organisation's shared set. If " + "you are an admin it is added directly; otherwise it becomes a " + "proposal for an admin to review. Accounts that aren't part of a " + "shared organisation don't have this workflow." + ), + ) + propose.add_argument("name", help="Skill name to share") + propose.add_argument( + "-m", + "--message", + default=None, + help="Optional message describing the change", + ) + + sync_parser.set_defaults(func=cmd_sync) diff --git a/tests/agent/test_org_skill_namespace.py b/tests/agent/test_org_skill_namespace.py new file mode 100644 index 0000000000000..b2f61e3ece8b6 --- /dev/null +++ b/tests/agent/test_org_skill_namespace.py @@ -0,0 +1,389 @@ +"""M2 org-skill namespace: token-gated resolution, provenance, collisions. + +Covers the design agreed 2026-07-23 (bare-name first-class org skills): + 1. TOKEN-GATED discovery — only the `.active_org`-marked mirror resolves; + stale mirrors and marker-less trees never load. + 2. Fail-loud collisions — a personal/org name clash lists BOTH sides flagged; + skill_view's existing multi-candidate guard refuses the bare name. + 3. Load-time provenance header — org skill content announces org + author. + 4. Org mirrors are read-only (skill_manage guards) and curation-exempt. +""" + +import json + +import pytest + +from agent import skill_utils as sku +from agent.prompt_builder import _build_snapshot_entry + + +def _mk_skill(root, rel, name=None, body="# body\n"): + d = root + for part in rel.split("/"): + d = d / part + d.mkdir(parents=True, exist_ok=True) + (d / "SKILL.md").write_text( + f"---\nname: {name or rel.split('/')[-1]}\ndescription: d\n---\n{body}", + encoding="utf-8", + ) + return d + + +def _mark_active(skills, org_id): + org_root = skills / sku.ORG_MIRROR_DIR_NAME + org_root.mkdir(parents=True, exist_ok=True) + (org_root / sku.ORG_ACTIVE_MARKER).write_text(org_id, encoding="utf-8") + + +class TestTokenGatedDiscovery: + def test_no_marker_no_org_skills(self, tmp_path): + skills = tmp_path / "skills" + _mk_skill(skills, "personal-a") + _mk_skill(skills, f"{sku.ORG_MIRROR_DIR_NAME}/org-1/shared-x", name="shared-x") + found = [p.parent.name for p in sku.iter_skill_index_files(skills, "SKILL.md")] + assert "personal-a" in found + assert "shared-x" not in found # unmarked mirror never resolves + + def test_marker_gates_to_active_org_only(self, tmp_path): + skills = tmp_path / "skills" + _mk_skill(skills, f"{sku.ORG_MIRROR_DIR_NAME}/org-1/shared-x", name="shared-x") + _mk_skill(skills, f"{sku.ORG_MIRROR_DIR_NAME}/org-OLD/stale-y", name="stale-y") + _mark_active(skills, "org-1") + found = [p.parent.name for p in sku.iter_skill_index_files(skills, "SKILL.md")] + assert "shared-x" in found + assert "stale-y" not in found # stale mirror pruned at resolution + + def test_switching_org_flips_resolution(self, tmp_path): + skills = tmp_path / "skills" + _mk_skill(skills, f"{sku.ORG_MIRROR_DIR_NAME}/org-1/shared-x", name="shared-x") + _mk_skill(skills, f"{sku.ORG_MIRROR_DIR_NAME}/org-2/other-z", name="other-z") + _mark_active(skills, "org-2") + found = [p.parent.name for p in sku.iter_skill_index_files(skills, "SKILL.md")] + assert found and "other-z" in found and "shared-x" not in found + + def test_helpers(self, tmp_path): + skills = tmp_path / "skills" + d = _mk_skill(skills, f"{sku.ORG_MIRROR_DIR_NAME}/org-9/cat/sk", name="sk") + assert sku.is_org_mirror_path(d, skills) is True + assert sku.org_id_of_path(d, skills) == "org-9" + p = _mk_skill(skills, "plain") + assert sku.is_org_mirror_path(p, skills) is False + assert sku.read_active_org_id(skills) is None + _mark_active(skills, "org-9") + assert sku.read_active_org_id(skills) == "org-9" + + +class TestSnapshotEntryProvenance: + def test_org_entry_strips_prefix_and_carries_provenance(self, tmp_path): + skills = tmp_path / "skills" + d = _mk_skill( + skills, f"{sku.ORG_MIRROR_DIR_NAME}/org-1/devops/beta", name="beta" + ) + (skills / sku.ORG_MIRROR_DIR_NAME / "org-1" / sku.ORG_PROVENANCE_FILE).write_text( + json.dumps( + {"author_device": "bens-macbook-a1b2c3", "author_user_id": "u1"} + ), + encoding="utf-8", + ) + entry = _build_snapshot_entry(d / "SKILL.md", skills, {"name": "beta"}, "d") + assert entry["org_id"] == "org-1" + assert entry["org_author"] == "bens-macbook-a1b2c3" + # Category derives from the path WITHIN the mirror, not _org/org-1/... + assert entry["category"] == "devops" + assert entry["skill_name"] == "beta" + + def test_personal_entry_unchanged(self, tmp_path): + skills = tmp_path / "skills" + d = _mk_skill(skills, "devops/beta", name="beta") + entry = _build_snapshot_entry(d / "SKILL.md", skills, {"name": "beta"}, "d") + assert "org_id" not in entry + assert entry["category"] == "devops" + + +class TestListingCollisionsAndLabels: + def _render(self, tmp_path, monkeypatch): + from agent import prompt_builder as pb + + skills = tmp_path / "skills" + skills.mkdir(parents=True, exist_ok=True) + monkeypatch.setattr(pb, "get_skills_dir", lambda: skills, raising=True) + monkeypatch.setattr( + pb, "get_all_skills_dirs", lambda: [skills], raising=True + ) + monkeypatch.setattr(pb, "get_disabled_skill_names", lambda *a, **k: set()) + monkeypatch.setattr( + pb, "_skills_prompt_snapshot_path", lambda: tmp_path / "snap.json" + ) + pb.clear_skills_system_prompt_cache() + return skills, pb + + def test_org_skill_listed_with_provenance_tag(self, tmp_path, monkeypatch): + skills, pb = self._render(tmp_path, monkeypatch) + _mk_skill(skills, "personal-a") + _mk_skill(skills, f"{sku.ORG_MIRROR_DIR_NAME}/org-1/shared-x", name="shared-x") + (skills / sku.ORG_MIRROR_DIR_NAME / "org-1" / sku.ORG_PROVENANCE_FILE).write_text( + json.dumps({"author_device": "bens-macbook"}), encoding="utf-8" + ) + _mark_active(skills, "org-1") + out = pb.build_skills_system_prompt() + assert "org:org-1" in out + assert "[org-shared: by bens-macbook]" in out + assert "personal-a" in out + + def test_collision_flags_both_sides(self, tmp_path, monkeypatch): + skills, pb = self._render(tmp_path, monkeypatch) + _mk_skill(skills, "k8s-debug", body="personal version\n") + _mk_skill( + skills, f"{sku.ORG_MIRROR_DIR_NAME}/org-1/k8s-debug", name="k8s-debug" + ) + _mark_active(skills, "org-1") + out = pb.build_skills_system_prompt() + # BOTH entries flagged — neither silently wins. + assert out.count("[name collision") == 2 + + def test_no_collision_flag_when_unique(self, tmp_path, monkeypatch): + skills, pb = self._render(tmp_path, monkeypatch) + _mk_skill(skills, "personal-a") + _mk_skill(skills, f"{sku.ORG_MIRROR_DIR_NAME}/org-1/shared-x", name="shared-x") + _mark_active(skills, "org-1") + out = pb.build_skills_system_prompt() + assert "[name collision" not in out + + +class TestOrgSkillsAreEditableInPlace: + """The learning loop must work ON shared skills, not around them. + + Refusing edits to `_org/` froze exactly the skills the most people use: + the agent is instructed to patch a skill the moment it finds a gap, and + "fork it to a personal skill first" is not something an agent does + mid-task. So edits land in place; org updates never clobber them; the + user (or auto-propose) shares them back. + """ + + def _org_skill(self, tmp_path, monkeypatch): + from tools import skill_manager_tool as smt + from agent import skill_utils as _sku + + skills = tmp_path / "skills" + d = _mk_skill( + skills, f"{sku.ORG_MIRROR_DIR_NAME}/org-1/shared-x", name="shared-x" + ) + _mark_active(skills, "org-1") + monkeypatch.setattr(smt, "_skills_dir", lambda: skills) + monkeypatch.setattr( + _sku, "get_all_skills_dirs", lambda: [skills], raising=True + ) + return smt, skills, d + + def test_patch_is_allowed_and_applied(self, tmp_path, monkeypatch): + smt, _skills, d = self._org_skill(tmp_path, monkeypatch) + result = smt._patch_skill("shared-x", "body", "improved") + assert result["success"] is True, result.get("error") + assert "improved" in (d / "SKILL.md").read_text(encoding="utf-8") + + def test_edit_tells_the_user_how_to_share_it_back(self, tmp_path, monkeypatch): + smt, _skills, _d = self._org_skill(tmp_path, monkeypatch) + result = smt._patch_skill("shared-x", "body", "improved") + # Without auto-propose the edit stays local, and the tool result must + # say so AND name the command — otherwise the improvement is stranded. + assert "propose" in (result.get("org_sharing") or "") + + def test_delete_is_still_refused(self, tmp_path, monkeypatch): + smt, _skills, d = self._org_skill(tmp_path, monkeypatch) + guard = smt._org_mirror_write_guard("shared-x", d, "delete") + assert guard is not None and guard["success"] is False + assert "admin" in guard["error"] + + def test_curation_is_allowed(self, tmp_path, monkeypatch): + from tools import skill_usage as su + + skills = tmp_path / "skills" + d = _mk_skill( + skills, f"{sku.ORG_MIRROR_DIR_NAME}/org-1/shared-x", name="shared-x" + ) + monkeypatch.setattr(su, "_skills_dir", lambda: skills) + # The curator must be able to improve shared skills — they are the + # highest-leverage ones in the system. + assert su.is_curation_eligible("shared-x", d) is True + + +class TestOrgPullIsWiredIn: + """Guards the integration gap that unit tests structurally cannot catch. + + The org pull functions were fully implemented and unit-tested while having + ZERO runtime callers, so org skills never loaded for anyone. Testing the + functions directly could never surface that. These tests assert the CALL + SITES exist, so the feature can't silently become dead code again. + """ + + def test_session_startup_calls_maybe_pull_org_skills(self): + import pathlib + + cli_src = ( + pathlib.Path(__file__).resolve().parents[2] / "cli.py" + ).read_text(encoding="utf-8") + assert "maybe_pull_org_skills" in cli_src, ( + "cli.py session startup must call maybe_pull_org_skills() — " + "without a call site the org mirror is never populated and org " + "skills never load (the function being importable is not enough)." + ) + # It must sit alongside the personal pull, not replace it. + assert "maybe_pull_skills" in cli_src + + def test_sync_pull_command_refreshes_org_mirror(self): + import pathlib + + main_src = ( + pathlib.Path(__file__).resolve().parents[2] + / "hermes_cli" + / "main.py" + ).read_text(encoding="utf-8") + assert "maybe_pull_org_skills" in main_src, ( + "`hermes sync pull` must also refresh the org mirror." + ) + + def test_sync_status_exposes_org_state(self): + from tools import skills_sync_client as ssc + + status = ssc.sync_status() + # These keys must always be present so a user can tell whether the org + # workflow applies to them, rather than it being invisible. + for key in ("org_available", "org_id", "org_role", "org_skills"): + assert key in status, f"sync status must expose {key!r}" + + def test_no_internal_jargon_in_user_facing_strings(self): + """User-visible help/errors must not leak internal design coordinates.""" + import pathlib + import re + + root = pathlib.Path(__file__).resolve().parents[2] + targets = [ + root / "hermes_cli" / "subcommands" / "sync.py", + root / "hermes_cli" / "subcommands" / "skills.py", + ] + banned = re.compile( + r"\(M[12]\)|\bHSP\b|HSP/1|§[0-9]|DEV-PHASE|hsp-1-contract" + ) + for path in targets: + for i, line in enumerate(path.read_text(encoding="utf-8").split("\n"), 1): + if "help=" in line or "description=" in line: + assert not banned.search(line), ( + f"{path.name}:{i} leaks internal jargon to users: {line.strip()}" + ) + + +class TestSkillSyncIsOneCommand: + """Every Skill Sync verb lives under `hermes sync` for launch. + + The surface is deliberately encapsulated: one command to learn, one to + document, and top-level `sync` stays free of skill-management verbs that + belong elsewhere. `propose` in particular used to sit under `hermes + skills`, which split one feature across two commands. + """ + + def _src(self, *parts): + import pathlib + + return ( + pathlib.Path(__file__).resolve().parents[2].joinpath(*parts) + ).read_text(encoding="utf-8") + + def test_propose_is_a_sync_subcommand(self): + sync_src = self._src("hermes_cli", "subcommands", "sync.py") + assert '"propose"' in sync_src, ( + "`propose` must be a `hermes sync` subcommand." + ) + + def test_propose_is_not_under_skills(self): + skills_src = self._src("hermes_cli", "subcommands", "skills.py") + assert '"propose"' not in skills_src, ( + "`propose` must NOT remain under `hermes skills` — Skill Sync is " + "one command for launch." + ) + + def test_sync_usage_lists_propose(self): + main_src = self._src("hermes_cli", "main.py") + usage_start = main_src.index("usage: hermes sync ") + usage_block = main_src[usage_start : usage_start + 1400] + assert "propose" in usage_block, ( + "`hermes sync` usage must list the propose verb." + ) + + +class TestLocalEditsSurviveOrgUpdates: + """Ben's requirement: local edits are never silently overwritten. + + An org pull materializes the shared set. Before this, it `rmtree`'d each + skill dir and re-wrote it, so any local improvement vanished on the next + session start with no warning. Now a locally-modified skill is skipped + and reported as a conflict for the user to resolve deliberately. + """ + + def _mirror(self, tmp_path, monkeypatch, body="original\n"): + from tools import skills_sync_client as ssc + + skills = tmp_path / "skills" + d = _mk_skill( + skills, + f"{sku.ORG_MIRROR_DIR_NAME}/org-1/shared-x", + name="shared-x", + body=body, + ) + _mark_active(skills, "org-1") + monkeypatch.setattr(ssc, "_skills_dir", lambda: skills) + monkeypatch.setattr( + ssc, "_org_dir", lambda: skills / sku.ORG_MIRROR_DIR_NAME + ) + return ssc, skills, d + + def test_unmodified_skill_is_not_flagged(self, tmp_path, monkeypatch): + ssc, _skills, d = self._mirror(tmp_path, monkeypatch) + ssc._write_org_baseline( + "org-1", + {"shared-x": {"fingerprint": ssc._skill_dir_fingerprint(d), "tree": "t1"}}, + ) + assert ssc.org_skill_is_locally_modified("shared-x", "org-1") is False + assert ssc.list_locally_modified_org_skills("org-1") == [] + + def test_edited_skill_is_detected(self, tmp_path, monkeypatch): + ssc, _skills, d = self._mirror(tmp_path, monkeypatch) + ssc._write_org_baseline( + "org-1", + {"shared-x": {"fingerprint": ssc._skill_dir_fingerprint(d), "tree": "t1"}}, + ) + (d / "SKILL.md").write_text("---\nname: shared-x\n---\nEDITED\n", encoding="utf-8") + assert ssc.org_skill_is_locally_modified("shared-x", "org-1") is True + assert ssc.list_locally_modified_org_skills("org-1") == ["shared-x"] + + def test_missing_baseline_does_not_cry_wolf(self, tmp_path, monkeypatch): + ssc, _skills, _d = self._mirror(tmp_path, monkeypatch) + # Mirror pulled before baselines existed — must not be reported as + # modified (that would block every update with a phantom conflict). + assert ssc.org_skill_is_locally_modified("shared-x", "org-1") is False + + def test_fingerprint_is_content_based_not_mtime(self, tmp_path, monkeypatch): + import os + import time + + ssc, _skills, d = self._mirror(tmp_path, monkeypatch) + before = ssc._skill_dir_fingerprint(d) + time.sleep(0.01) + os.utime(d / "SKILL.md", None) # touch: mtime changes, content doesn't + assert ssc._skill_dir_fingerprint(d) == before + + def test_auto_propose_defaults_off(self, monkeypatch): + from tools import skills_sync_client as ssc + + monkeypatch.delenv("HERMES_SYNC_ORG_AUTO_PROPOSE", raising=False) + monkeypatch.setattr( + "hermes_cli.config.load_config", lambda: {}, raising=False + ) + # Default must be OFF: silently pushing every agent edit to the whole + # organisation is not a safe default. + assert ssc.sync_org_auto_propose() is False + + def test_auto_propose_can_be_enabled_by_env(self, monkeypatch): + from tools import skills_sync_client as ssc + + monkeypatch.setenv("HERMES_SYNC_ORG_AUTO_PROPOSE", "1") + assert ssc.sync_org_auto_propose() is True diff --git a/tests/tools/test_skills_sync_client.py b/tests/tools/test_skills_sync_client.py new file mode 100644 index 0000000000000..ec7e68d933126 --- /dev/null +++ b/tests/tools/test_skills_sync_client.py @@ -0,0 +1,978 @@ +"""Tests for tools/skills_sync_client.py — the Skill Sync client. + +Covers, against the frozen contract (~/src/specs/collective-wisdom/ +the sync wire contract): + * content addressing (full 64-hex) + canonical JSON (§2.1, §2.5) + * the access gate (Nous admin) making sync inert + * the M1-D opt-in default (nothing syncs without the sync flag) + * object building (blob/tree/commit, exec mode, size limit) + * push (upload + CAS), pull (materialize), and the three-way merge / 409 + conflict paths — all against an in-process mock sync server. + +The mock server implements the contract §3/§4 endpoint shapes with an +in-memory object store + ref table. No live server, no network. +""" + +import hashlib +import json +import threading +from http.server import BaseHTTPRequestHandler, HTTPServer +from pathlib import Path + +import pytest + +import tools.skills_sync_client as ssc + + +# --------------------------------------------------------------------------- +# In-process mock sync server (read + write endpoints) +# --------------------------------------------------------------------------- + +class _MockState: + def __init__(self): + self.objects = {} # hash -> (kind, bytes) + self.refs = {} # name -> commit hash + self.hsp_version = "1" + self.max_object_bytes = 26214400 + self.force_conflict_once = False # inject a 409 on the next CAS + # M2 org behavior (contract §11): advertise the "org" feature and, + # when org_role_admin is False, convert org-HEAD CAS to 202 proposals. + self.org_feature = True + self.org_role_admin = True + self.proposals = [] # [{n, to, base}] + + +def _make_handler(state: _MockState): + class Handler(BaseHTTPRequestHandler): + def log_message(self, format, *args): # silence + pass + + def _json(self, code, obj, extra_headers=None): + body = json.dumps(obj).encode("utf-8") + self.send_response(code) + self.send_header("Content-Type", "application/json") + for k, v in (extra_headers or {}).items(): + self.send_header(k, v) + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + + def do_GET(self): + path = self.path.split("?", 1)[0] + query = "" + if "?" in self.path: + query = self.path.split("?", 1)[1] + + if path == "/v1/sync/capabilities": + features = ["personal"] + (["org"] if state.org_feature else []) + return self._json(200, { + "hsp_version": state.hsp_version, + "features": features, + "max_object_bytes": state.max_object_bytes, + "hash_alg": "sha256", + "auth": "bearer", + }) + + if path == "/v1/sync/refs": + prefix = "" + for part in query.split("&"): + if part.startswith("prefix="): + from urllib.parse import unquote + prefix = unquote(part[len("prefix="):]) + refs = [ + {"name": n, "hash": h} + for n, h in state.refs.items() + if n.startswith(prefix) + ] + return self._json(200, {"refs": refs}) + + if path.startswith("/v1/sync/objects/"): + obj_hash = path[len("/v1/sync/objects/"):] + if obj_hash not in state.objects: + return self._json(404, {"error": "not_found"}) + kind, data = state.objects[obj_hash] + if kind == ssc.KIND_BLOB: + self.send_response(200) + self.send_header("Content-Type", "application/octet-stream") + self.send_header("X-HSP-Object-Type", "blob") + self.send_header("Content-Length", str(len(data))) + self.end_headers() + self.wfile.write(data) + return + self.send_response(200) + self.send_header("Content-Type", "application/json") + self.send_header("X-HSP-Object-Type", kind) + self.send_header("Content-Length", str(len(data))) + self.end_headers() + self.wfile.write(data) + return + + self._json(404, {"error": "unknown"}) + + def do_POST(self): + length = int(self.headers.get("Content-Length", 0)) + raw = self.rfile.read(length) if length else b"" + path = self.path.split("?", 1)[0] # e.g. /v1/sync/objects?scope=org + + if path == "/v1/sync/objects": + return self._handle_put_objects(raw) + + if path.startswith("/v1/sync/refs/"): + return self._handle_cas(raw) + + self._json(404, {"error": "unknown"}) + + def _handle_put_objects(self, raw): + # multipart/form-data: parse parts (field=hash, filename=type, + # body=raw bytes). The server recomputes each hash and 422s on + # mismatch (contract §4.2). + ctype = self.headers.get("Content-Type", "") + if "multipart/form-data" not in ctype: + return self._json(400, {"error": "expected multipart"}) + boundary = ctype.split("boundary=", 1)[1].encode("ascii") + accepted, already = [], [] + parts = raw.split(b"--" + boundary) + for part in parts: + # Only trim the delimiter framing: a leading CRLF and a + # trailing CRLF. Do NOT strip() the whole part -- that would + # also eat legitimate trailing newlines from the object bytes. + if part.startswith(b"\r\n"): + part = part[2:] + if part.endswith(b"\r\n"): + part = part[:-2] + if not part or part == b"--": + continue + if b"\r\n\r\n" not in part: + continue + headers_blob, body = part.split(b"\r\n\r\n", 1) + hdr_text = headers_blob.decode("utf-8", "replace") + claimed_hash = None + kind = None + for line in hdr_text.split("\r\n"): + if line.lower().startswith("content-disposition"): + for token in line.split(";"): + token = token.strip() + if token.startswith('name="'): + claimed_hash = token[len('name="'):-1] + elif token.startswith('filename="'): + kind = token[len('filename="'):-1] + if claimed_hash is None: + continue + real = "sha256:" + hashlib.sha256(body).hexdigest() + if real != claimed_hash: + return self._json(422, { + "error": "hash_mismatch", "claimed": claimed_hash, + }) + if claimed_hash in state.objects: + already.append(claimed_hash) + else: + state.objects[claimed_hash] = (kind, body) + accepted.append(claimed_hash) + return self._json(200, {"accepted": accepted, "already_present": already}) + + def _handle_cas(self, raw): + from urllib.parse import unquote + name = unquote(self.path[len("/v1/sync/refs/"):]) + body = json.loads(raw.decode("utf-8")) if raw else {} + frm = body.get("from") + to = body.get("to") + # M2 (contract §11.5): a non-admin member's CAS on an org HEAD is + # accept-always converted to a proposal → 202. + if name.startswith("refs/org/") and not state.org_role_admin: + n = len(state.proposals) + 1 + state.proposals.append({"n": n, "to": to, "base": frm}) + org = name.split("/")[2] + prop_ref = f"refs/org/{org}/proposals/{n}" + state.refs[prop_ref] = to + return self._json(202, {"proposal_id": n, "ref": prop_ref}) + if state.force_conflict_once: + state.force_conflict_once = False + return self._json(409, {"actual": state.refs.get(name, "")}) + current = state.refs.get(name) + if current != frm: + return self._json(409, {"actual": current or ""}) + state.refs[name] = to + return self._json(200, {"ref": name, "hash": to}) + + return Handler + + +@pytest.fixture +def mock_server(): + state = _MockState() + server = HTTPServer(("127.0.0.1", 0), _make_handler(state)) + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + base = f"http://127.0.0.1:{server.server_address[1]}" + try: + yield base, state + finally: + server.shutdown() + server.server_close() + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +def _write_skill(skills_dir: Path, name: str, body: str = "# skill\n", *, category=None): + """Create a minimal skill dir under skills_dir; return its path.""" + parent = skills_dir / category if category else skills_dir + d = parent / name + d.mkdir(parents=True, exist_ok=True) + (d / "SKILL.md").write_text( + f"---\nname: {name}\ndescription: test\n---\n{body}", encoding="utf-8" + ) + return d + + +def _jwt(claims: dict) -> str: + import jwt as _pyjwt + return _pyjwt.encode(claims, "x" * 32, algorithm="HS256") + + +# --------------------------------------------------------------------------- +# Content addressing & canonicalization (contract §2.1, §2.5, OI-5) +# --------------------------------------------------------------------------- + +class TestAddressing: + def test_full_64_hex_address(self): + addr = ssc.wire_address(b"") + # sha256 of empty is the well-known e3b0... digest, full 64 hex. + assert addr == ( + "sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" + ) + assert len(addr.split(":", 1)[1]) == 64 + + def test_address_differs_from_local_truncated_namespace(self): + # The wire full-64-hex must NOT equal the local truncated 16-hex form. + data = b"hello world" + full = ssc.wire_address(data) + truncated = "sha256:" + hashlib.sha256(data).hexdigest()[:16] + assert full != truncated + assert len(full.split(":")[1]) == 64 + assert len(truncated.split(":")[1]) == 16 + + def test_canonical_json_sorted_no_whitespace(self): + out = ssc.canonical_json_bytes({"b": 1, "a": 2}) + assert out == b'{"a":2,"b":1}' + assert b" " not in out + assert not out.endswith(b"\n") + + def test_canonical_json_stable(self): + obj = {"type": "tree", "entries": [{"name": "x", "hash": "sha256:aa"}]} + assert ssc.canonical_json_bytes(obj) == ssc.canonical_json_bytes(dict(obj)) + + +# --------------------------------------------------------------------------- +# Access gate (Nous admin) + per-skill opt-in +# --------------------------------------------------------------------------- + +class TestDevGate: + def test_gate_open_with_claim(self, monkeypatch): + token = _jwt({"sub": "user1", "tool_gateway_admin": True}) + monkeypatch.setattr( + ssc, "resolve_nous_runtime_credentials", + lambda **kw: {"api_key": token, "base_url": "https://x"}, raising=False, + ) + # patch the lazily-imported symbol used inside resolve_identity + import hermes_cli.auth as auth_mod + monkeypatch.setattr(auth_mod, "resolve_nous_runtime_credentials", + lambda **kw: {"api_key": token, "base_url": "https://x"}) + ident = ssc.resolve_identity() + assert ident["nous_admin"] is True + assert ident["owner"] == "user1" + + def test_gate_closed_without_claim(self, monkeypatch): + token = _jwt({"sub": "user1"}) # no tool_gateway_admin + import hermes_cli.auth as auth_mod + monkeypatch.setattr(auth_mod, "resolve_nous_runtime_credentials", + lambda **kw: {"api_key": token, "base_url": "https://x"}) + ident = ssc.resolve_identity() + assert ident["nous_admin"] is False + + def test_gate_closed_when_claim_false(self, monkeypatch): + token = _jwt({"sub": "u", "tool_gateway_admin": False}) + import hermes_cli.auth as auth_mod + monkeypatch.setattr(auth_mod, "resolve_nous_runtime_credentials", + lambda **kw: {"api_key": token, "base_url": "https://x"}) + assert ssc.dev_gate_open() is False + + def test_maybe_push_inert_when_gate_closed(self, monkeypatch): + token = _jwt({"sub": "u"}) + import hermes_cli.auth as auth_mod + monkeypatch.setattr(auth_mod, "resolve_nous_runtime_credentials", + lambda **kw: {"api_key": token}) + monkeypatch.setattr(ssc, "resolve_sync_base_url", lambda: "http://x") + # gate closed -> None (inert), never attempts a push + assert ssc.maybe_push_skills() is None + + def test_maybe_pull_inert_when_not_logged_in(self, monkeypatch): + import hermes_cli.auth as auth_mod + + def _raise(**kw): + raise RuntimeError("not logged in") + + monkeypatch.setattr(auth_mod, "resolve_nous_runtime_credentials", _raise) + assert ssc.maybe_pull_skills() is None + + +# --------------------------------------------------------------------------- +# Object building (contract §2.2-§2.4) +# --------------------------------------------------------------------------- + +class TestObjectBuilding: + def test_build_tree_blob_and_exec(self, tmp_path): + d = tmp_path / "skill" + d.mkdir() + (d / "SKILL.md").write_text("hello", encoding="utf-8") + script = d / "run.sh" + script.write_text("#!/bin/sh\necho hi\n", encoding="utf-8") + script.chmod(0o755) + + objects = ssc.ObjectSet() + tree_hash = ssc.build_tree(d, objects, max_object_bytes=ssc.DEFAULT_MAX_OBJECT_BYTES) + assert tree_hash.startswith("sha256:") + # tree object present and canonical + kind, data = objects.objects[tree_hash] + assert kind == ssc.KIND_TREE + tree = json.loads(data) + entries = {e["name"]: e for e in tree["entries"]} + assert entries["SKILL.md"]["mode"] == ssc.MODE_FILE + assert entries["run.sh"]["mode"] == ssc.MODE_EXEC + # entries sorted by name (byte order) + names = [e["name"] for e in tree["entries"]] + assert names == sorted(names) + + def test_build_tree_dedups_identical_blobs(self, tmp_path): + d = tmp_path / "skill" + (d / "a").mkdir(parents=True) + (d / "b").mkdir(parents=True) + (d / "a" / "f.txt").write_text("same", encoding="utf-8") + (d / "b" / "f.txt").write_text("same", encoding="utf-8") + objects = ssc.ObjectSet() + ssc.build_tree(d, objects, max_object_bytes=ssc.DEFAULT_MAX_OBJECT_BYTES) + blob_hashes = [h for h, (k, _) in objects.objects.items() if k == ssc.KIND_BLOB] + # only one unique blob for the identical "same" content + assert len(set(blob_hashes)) == 1 + + def test_build_tree_skips_symlink(self, tmp_path): + d = tmp_path / "skill" + d.mkdir() + (d / "real.txt").write_text("x", encoding="utf-8") + try: + (d / "link.txt").symlink_to(d / "real.txt") + except (OSError, NotImplementedError): + pytest.skip("symlinks unsupported here") + objects = ssc.ObjectSet() + tree_hash = ssc.build_tree(d, objects, max_object_bytes=ssc.DEFAULT_MAX_OBJECT_BYTES) + tree = json.loads(objects.objects[tree_hash][1]) + names = [e["name"] for e in tree["entries"]] + assert "link.txt" not in names + assert "real.txt" in names + + def test_build_tree_rejects_oversize_blob(self, tmp_path): + d = tmp_path / "skill" + d.mkdir() + (d / "big").write_bytes(b"x" * 100) + objects = ssc.ObjectSet() + with pytest.raises(ValueError): + ssc.build_tree(d, objects, max_object_bytes=10) + + def test_build_commit_shape(self): + objects = ssc.ObjectSet() + c = ssc.build_commit( + "sha256:tree", ["sha256:p"], owner="o", device="dev", + message="m", objects=objects, ts="2026-07-18T00:00:00Z", + ) + commit = json.loads(objects.objects[c][1]) + assert commit["type"] == "commit" + assert commit["tree"] == "sha256:tree" + assert commit["parents"] == ["sha256:p"] + assert commit["author"] == {"owner": "o", "device": "dev"} + assert commit["artifact_type"] == "skill" + + +# --------------------------------------------------------------------------- +# Three-way merge decision (contract §4.4, M1-C; mirrors skills_sync.py:619) +# --------------------------------------------------------------------------- + +class TestMergeDecision: + def test_no_change(self): + assert ssc._merge_skill("b", "b", "b") == "either" + + def test_ours_only_changed(self): + assert ssc._merge_skill("b", "o", "b") == "ours" + + def test_theirs_only_changed(self): + assert ssc._merge_skill("b", "b", "t") == "theirs" + + def test_both_converged(self): + assert ssc._merge_skill("b", "x", "x") == "either" + + def test_true_overlap(self): + assert ssc._merge_skill("b", "o", "t") == "overlap" + + def test_deleted_both(self): + assert ssc._merge_skill(None, None, None) == "none" + + +# --------------------------------------------------------------------------- +# End-to-end push / pull / conflict against the mock server +# --------------------------------------------------------------------------- + +@pytest.fixture +def synced_env(tmp_path, monkeypatch): + """A HERMES_HOME with two opted-in skills + a token-carrying identity.""" + import hermes_constants + home = tmp_path / "hermes" + skills = home / "skills" + skills.mkdir(parents=True) + monkeypatch.setattr(hermes_constants, "get_hermes_home", lambda: home) + monkeypatch.setattr(ssc, "_skills_dir", lambda: skills) + + _write_skill(skills, "alpha", body="alpha v1\n") + _write_skill(skills, "beta", body="beta v1\n", category="devops") + + # Opt both into sync + treat them as eligible (bypass bundled/hub checks). + monkeypatch.setattr(ssc, "list_synced_skill_names", lambda: ["alpha", "beta"]) + + def _rel(name): + from pathlib import PurePosixPath + return {"alpha": PurePosixPath("alpha"), + "beta": PurePosixPath("devops/beta")}.get(name) + + monkeypatch.setattr(ssc, "_skill_rel_path", _rel) + + def _find(name): + return {"alpha": skills / "alpha", + "beta": skills / "devops" / "beta"}.get(name) + + import tools.skill_usage as su + monkeypatch.setattr(su, "_find_skill_dir", _find) + + token = _jwt({"sub": "owner1", "tool_gateway_admin": True}) + identity = {"api_key": token, "base_url": "http://x", "owner": "owner1", + "nous_admin": True, "claims": {}} + return home, skills, identity + + +class TestEndToEnd: + def test_capabilities_version_check(self, mock_server): + base, state = mock_server + client = ssc.SyncClient(base, "tok") + caps = client.capabilities() + assert caps["hsp_version"] == "1" + ssc._check_version(caps) # no raise + + def test_version_mismatch_raises(self, mock_server): + base, state = mock_server + state.hsp_version = "2" + client = ssc.SyncClient(base, "tok") + with pytest.raises(ssc.SyncError): + ssc._check_version(client.capabilities()) + + def test_push_uploads_and_cas(self, mock_server, synced_env): + base, state = mock_server + home, skills, identity = synced_env + client = ssc.SyncClient(base, identity["api_key"]) + result = ssc.push_skills(client, identity=identity) + assert result["ok"] is True + # HEAD ref advanced to our commit + head = state.refs["refs/user/owner1/HEAD"] + assert head == result["head"] + # commit object is present and well-formed + kind, data = state.objects[head] + assert kind == ssc.KIND_COMMIT + commit = json.loads(data) + assert commit["author"]["owner"] == "owner1" + assert commit["parents"] == [] # first commit + + def test_push_then_pull_materializes(self, mock_server, synced_env, tmp_path, monkeypatch): + base, state = mock_server + home, skills, identity = synced_env + client = ssc.SyncClient(base, identity["api_key"]) + ssc.push_skills(client, identity=identity) + + # Simulate a fresh device: new skills dir, same server, same opt-in. + dev2 = tmp_path / "hermes2" / "skills" + dev2.mkdir(parents=True) + monkeypatch.setattr(ssc, "_skills_dir", lambda: dev2) + monkeypatch.setattr(ssc, "read_sync_state", lambda: {"head": None, "skills": {}}) + saved = {} + monkeypatch.setattr(ssc, "write_sync_state", lambda d: saved.update(d)) + + result = ssc.pull_skills(client, identity=identity) + assert result["ok"] is True + assert "alpha" in result["updated"] + assert "devops/beta" in result["updated"] + # content materialized to disk + assert (dev2 / "alpha" / "SKILL.md").read_text().endswith("alpha v1\n") + assert (dev2 / "devops" / "beta" / "SKILL.md").read_text().endswith("beta v1\n") + + def test_push_idempotent_reupload(self, mock_server, synced_env): + base, state = mock_server + home, skills, identity = synced_env + client = ssc.SyncClient(base, identity["api_key"]) + r1 = ssc.push_skills(client, identity=identity) + n_objects = len(state.objects) + # push again with no local change -> same head, objects already_present + r2 = ssc.push_skills(client, identity=identity) + assert r2["ok"] is True + assert r2["head"] == r1["head"] + assert len(state.objects) == n_objects # nothing new stored + + def test_conflict_nonoverlap_merges(self, mock_server, synced_env, monkeypatch): + base, state = mock_server + home, skills, identity = synced_env + client = ssc.SyncClient(base, identity["api_key"]) + # First push establishes a base head we record locally. + first = ssc.push_skills(client, identity=identity) + # Inject a divergent server head: change beta server-side so the next + # CAS loses. We simulate by forcing one 409 whose actual == current head + # (the server keeps the same tree, so no overlap on alpha which we edit). + (skills / "alpha" / "SKILL.md").write_text( + "---\nname: alpha\ndescription: test\n---\nalpha v2\n", encoding="utf-8" + ) + state.force_conflict_once = True + result = ssc.push_skills(client, identity=identity) + # actual == our own head -> both-sides identical -> merge commit succeeds + assert result.get("ok") is True + assert result.get("merged") is True + + def test_conflict_true_overlap_writes_conflict_ref(self, mock_server, synced_env, monkeypatch): + base, state = mock_server + home, skills, identity = synced_env + client = ssc.SyncClient(base, identity["api_key"]) + ssc.push_skills(client, identity=identity) + + # Build a DIFFERENT server-side head for the SAME skill (alpha) so the + # three-way merge sees a true overlap. We construct it via a second + # snapshot after editing alpha differently, push it directly, then make + # our local head stale and edit alpha a third way. + (skills / "alpha" / "SKILL.md").write_text( + "---\nname: alpha\ndescription: test\n---\nSERVER edit\n", encoding="utf-8" + ) + objs, root, _ = ssc.snapshot_profile(["alpha", "beta"]) + their_commit = ssc.build_commit( + root, [], owner="owner1", device="other", message="theirs", objects=objs + ) + client.put_objects(objs.objects) + state.refs["refs/user/owner1/HEAD"] = their_commit + + # Our local edit to the same skill, from the OLD base -> true overlap. + (skills / "alpha" / "SKILL.md").write_text( + "---\nname: alpha\ndescription: test\n---\nLOCAL edit\n", encoding="utf-8" + ) + result = ssc.push_skills(client, identity=identity) + assert result.get("conflict") is True + assert result["conflict_ref"].startswith("refs/user/owner1/conflict/") + assert "alpha" in result["overlapping_skills"] + # a conflict ref head was written server-side + assert result["conflict_ref"] in state.refs + + +# --------------------------------------------------------------------------- +# M1-D opt-in sidecar flag (tools/skill_usage.set_sync / is_sync_enabled) +# --------------------------------------------------------------------------- + +class TestOptInFlag: + def test_set_and_read_sync_flag(self, tmp_path, monkeypatch): + import tools.skill_usage as su + monkeypatch.setattr(su, "_skills_dir", lambda: tmp_path) + # Make the skill curation-eligible so the gated mutator writes. + monkeypatch.setattr(su, "is_curation_eligible", lambda name, *a, **k: True) + + assert su.is_sync_enabled("foo") is False + su.set_sync("foo", True) + assert su.is_sync_enabled("foo") is True + su.set_sync("foo", False) + assert su.is_sync_enabled("foo") is False + + def test_sync_flag_ignored_for_ineligible(self, tmp_path, monkeypatch): + import tools.skill_usage as su + monkeypatch.setattr(su, "_skills_dir", lambda: tmp_path) + # Bundled/hub/external skills are not curation-eligible -> mutator no-ops. + monkeypatch.setattr(su, "is_curation_eligible", lambda name, *a, **k: False) + su.set_sync("bundled-skill", True) + assert su.is_sync_enabled("bundled-skill") is False + + +# --------------------------------------------------------------------------- +# §2.8 sync-manifest — opt-in as content in the sync plane (cross-device) +# --------------------------------------------------------------------------- + +class TestSyncManifest: + def test_build_parse_roundtrip(self): + data = ssc.build_sync_manifest_bytes({"beta": True, "alpha": False}) + parsed = ssc.parse_sync_manifest(data) + assert parsed == {"alpha": False, "beta": True} + + def test_manifest_wire_shape(self): + # Must match gateway-gateway src/sync/manifest.ts: type + version:1 + + # skills:[{name,enabled}]. Skills sorted by name for a stable address. + import json + data = ssc.build_sync_manifest_bytes({"z": True, "a": True}) + obj = json.loads(data.decode("utf-8")) + assert obj["type"] == "sync-manifest" + assert obj["version"] == 1 + assert obj["skills"] == [ + {"name": "a", "enabled": True}, + {"name": "z", "enabled": True}, + ] + + def test_parse_rejects_malformed(self): + # Strict: unknown type, bad version, non-array skills, malformed entry. + assert ssc.parse_sync_manifest(b"not json") is None + assert ssc.parse_sync_manifest(b'{"type":"nope","version":1,"skills":[]}') is None + assert ssc.parse_sync_manifest(b'{"type":"sync-manifest","version":2,"skills":[]}') is None + assert ssc.parse_sync_manifest(b'{"type":"sync-manifest","version":1,"skills":{}}') is None + assert ( + ssc.parse_sync_manifest( + b'{"type":"sync-manifest","version":1,"skills":[{"name":"x"}]}' + ) + is None + ) + # A malformed manifest must NOT be mistaken for "no skills opted in". + assert ssc.parse_sync_manifest(b'{"type":"sync-manifest","version":1,"skills":[]}') == {} + + def test_snapshot_embeds_manifest_root_blob(self, mock_server, synced_env): + # snapshot_profile must add a root-level `sync-manifest` blob recording + # the opted-in set, alongside the skill subtrees, so opt-in is durable + # plane content. Read it back via read_manifest_of_root. + base, state = mock_server + home, skills, identity = synced_env + client = ssc.SyncClient(base, identity["api_key"]) + + objs, root_hash, skill_map = ssc.snapshot_profile(["alpha", "beta"]) + client.put_objects(objs.objects) + + manifest = ssc.read_manifest_of_root(client, root_hash) + assert manifest == {"alpha": True, "beta": True} + + # The manifest is a root-level BLOB, not a skill subtree, so the skill + # walk must not surface it as a skill. + trees = ssc._skill_trees_of_root(client, root_hash) + assert "sync-manifest" not in trees + assert set(trees) == {"alpha", "devops/beta"} + + def test_pull_adopts_opt_in_from_manifest(self, mock_server, synced_env, monkeypatch): + # A skill opted in on device A (present + enabled in the plane manifest) + # becomes opted in locally on pull, even if this device had it disabled. + base, state = mock_server + home, skills, identity = synced_env + client = ssc.SyncClient(base, identity["api_key"]) + + # Device A pushes alpha+beta (manifest enables both). + ssc.push_skills(client, identity=identity) + + # Simulate device B: local opt-in intent is EMPTY, but eligibility passes. + adopted = {} + import tools.skill_usage as su + monkeypatch.setattr(su, "is_curation_eligible", lambda name, *a, **k: True) + monkeypatch.setattr(su, "is_sync_enabled", lambda name: False) + monkeypatch.setattr(su, "set_sync", lambda name, val: adopted.__setitem__(name, val)) + # Local head unknown so the pull actually runs. + monkeypatch.setattr(ssc, "read_sync_state", lambda: {"head": None, "skills": {}}) + monkeypatch.setattr(ssc, "write_sync_state", lambda d: None) + # No local opt-in gate (so materialize isn't the thing under test). + monkeypatch.setattr(ssc, "_opted_in_rel_paths", lambda: []) + + result = ssc.pull_skills(client, identity=identity) + assert result["ok"] is True + # Both skills from the plane manifest were adopted into local opt-in. + assert adopted == {"alpha": True, "beta": True} + assert set(result["opt_in_adopted"]) == {"alpha", "beta"} + + +# --------------------------------------------------------------------------- +# Env-var configuration (Hermes Cloud "on by default" via environment) +# --------------------------------------------------------------------------- + +class TestEnvConfig: + def test_base_url_env_wins(self, monkeypatch): + monkeypatch.setenv("HERMES_SYNC_BASE_URL", "https://plane.example/") + assert ssc.resolve_sync_base_url() == "https://plane.example" + + def test_base_url_defaults_to_production(self, monkeypatch): + # With nothing configured a user must still reach the real plane — + # otherwise every sync command fails with "no base URL configured". + monkeypatch.delenv("HERMES_SYNC_BASE_URL", raising=False) + monkeypatch.setattr("hermes_cli.config.load_config", lambda: {}, raising=False) + assert ssc.resolve_sync_base_url() == ssc.DEFAULT_SYNC_BASE_URL + + def test_default_is_a_bare_https_origin(self): + # The client appends /v1/sync/, so the default must be a scheme+host + # origin with no trailing slash and no path. + from urllib.parse import urlparse + + parsed = urlparse(ssc.DEFAULT_SYNC_BASE_URL) + assert parsed.scheme == "https" + assert parsed.netloc + assert parsed.path == "" + assert not ssc.DEFAULT_SYNC_BASE_URL.endswith("/") + + def test_config_overrides_default(self, monkeypatch): + monkeypatch.delenv("HERMES_SYNC_BASE_URL", raising=False) + monkeypatch.setattr( + "hermes_cli.config.load_config", + lambda: {"sync": {"base_url": "https://cfg.example/"}}, + raising=False, + ) + assert ssc.resolve_sync_base_url() == "https://cfg.example" + + def test_feature_enabled_env(self, monkeypatch): + # Default off. + monkeypatch.delenv("HERMES_SYNC_ENABLED", raising=False) + monkeypatch.setattr("hermes_cli.config.load_config", lambda: {}, raising=False) + assert ssc.sync_feature_enabled() is False + for truthy in ("1", "true", "YES", "on"): + monkeypatch.setenv("HERMES_SYNC_ENABLED", truthy) + assert ssc.sync_feature_enabled() is True + for falsy in ("0", "false", "off"): + monkeypatch.setenv("HERMES_SYNC_ENABLED", falsy) + assert ssc.sync_feature_enabled() is False + + def test_default_opt_in_env(self, monkeypatch): + monkeypatch.delenv("HERMES_SYNC_DEFAULT_OPT_IN", raising=False) + monkeypatch.setattr("hermes_cli.config.load_config", lambda: {}, raising=False) + assert ssc.sync_default_opt_in() is False + monkeypatch.setenv("HERMES_SYNC_DEFAULT_OPT_IN", "true") + assert ssc.sync_default_opt_in() is True + + def test_config_yaml_fallback_when_no_env(self, monkeypatch): + monkeypatch.delenv("HERMES_SYNC_ENABLED", raising=False) + monkeypatch.setattr( + "hermes_cli.config.load_config", + lambda: {"sync": {"enabled": True}}, + raising=False, + ) + assert ssc.sync_feature_enabled() is True + + def test_env_overrides_config_yaml(self, monkeypatch): + # Env wins over config.yaml (operator override precedence). + monkeypatch.setenv("HERMES_SYNC_ENABLED", "false") + monkeypatch.setattr( + "hermes_cli.config.load_config", + lambda: {"sync": {"enabled": True}}, + raising=False, + ) + assert ssc.sync_feature_enabled() is False + + def test_opt_out_policy_syncs_all_eligible(self, monkeypatch): + # With opt-out on, every eligible skill syncs even with no `sync:true` + # flag; an explicit `sync:false` still excludes. + monkeypatch.setattr(ssc, "sync_default_opt_in", lambda: True) + monkeypatch.setattr(ssc, "_all_local_skill_names", lambda: ["alpha", "beta", "gamma"]) + monkeypatch.setattr(ssc, "is_sync_eligible", lambda n: n in {"alpha", "beta", "gamma"}) + import tools.skill_usage as su + # gamma explicitly opted out; alpha/beta have no flag. + monkeypatch.setattr(su, "load_usage", lambda: {"gamma": {"sync": False}}) + assert ssc.list_synced_skill_names() == ["alpha", "beta"] + + def test_opt_in_policy_requires_flag(self, monkeypatch): + # With opt-out OFF (default opt-in), only explicitly-enabled skills sync. + monkeypatch.setattr(ssc, "sync_default_opt_in", lambda: False) + monkeypatch.setattr(ssc, "is_sync_eligible", lambda n: True) + import tools.skill_usage as su + monkeypatch.setattr( + su, "load_usage", + lambda: {"alpha": {"sync": True}, "beta": {}, "gamma": {"sync": False}}, + ) + assert ssc.list_synced_skill_names() == ["alpha"] + + +class TestDeviceName: + def test_default_is_hostname_seeded(self, tmp_path, monkeypatch): + monkeypatch.setattr(ssc, "_skills_dir", lambda: tmp_path) + monkeypatch.delenv("HERMES_SYNC_DEVICE_NAME", raising=False) + monkeypatch.setattr( + "socket.gethostname", lambda: "bens-macbook.local", raising=False + ) + val = ssc.stable_device_id() + # short hostname + short suffix, NOT a bare 32-char hash + assert val.startswith("bens-macbook-") + assert val != "bens-macbook-" + # persisted + stable across calls + assert (tmp_path / ".sync_device_id").read_text() == val + assert ssc.stable_device_id() == val + + def test_existing_file_wins_over_default_and_env(self, tmp_path, monkeypatch): + monkeypatch.setattr(ssc, "_skills_dir", lambda: tmp_path) + (tmp_path / ".sync_device_id").write_text("Explicit Name", encoding="utf-8") + monkeypatch.setenv("HERMES_SYNC_DEVICE_NAME", "cloud-seed") + assert ssc.stable_device_id() == "Explicit Name" + + def test_env_seeds_first_use(self, tmp_path, monkeypatch): + # Hermes Cloud path: HERMES_SYNC_DEVICE_NAME seeds the first-use label. + monkeypatch.setattr(ssc, "_skills_dir", lambda: tmp_path) + monkeypatch.setenv("HERMES_SYNC_DEVICE_NAME", "hermes-cloud-ben-1") + assert ssc.stable_device_id() == "hermes-cloud-ben-1" + # persisted so it stays stable even if the env later changes + assert (tmp_path / ".sync_device_id").read_text() == "hermes-cloud-ben-1" + monkeypatch.setenv("HERMES_SYNC_DEVICE_NAME", "changed") + assert ssc.stable_device_id() == "hermes-cloud-ben-1" + + def test_set_device_name_overwrites(self, tmp_path, monkeypatch): + monkeypatch.setattr(ssc, "_skills_dir", lambda: tmp_path) + (tmp_path / ".sync_device_id").write_text("old", encoding="utf-8") + stored = ssc.set_device_name(" Ben's Laptop ") + assert stored == "Ben's Laptop" # trimmed + assert ssc.stable_device_id() == "Ben's Laptop" + + def test_set_device_name_rejects_empty(self, tmp_path, monkeypatch): + monkeypatch.setattr(ssc, "_skills_dir", lambda: tmp_path) + import pytest + + with pytest.raises(ValueError): + ssc.set_device_name(" ") + + +# --------------------------------------------------------------------------- +# M2 org-shared skills (contract §11): identity gate, pull, propose (202/merge) +# --------------------------------------------------------------------------- + +def _org_identity(role=None, org_id="org-1", owner="owner1"): + claims = {"sub": owner, "org_id": org_id, "tool_gateway_admin": True} + if role is not None: + claims["org_role"] = role + token = _jwt(claims) + return {"api_key": token, "base_url": "http://x", "owner": owner, + "nous_admin": True, "claims": claims, + **({"org_id": org_id, "org_role": role} if role else {})} + + +class TestOrgIdentityGate: + def test_org_identity_requires_role_claim(self, monkeypatch): + # Personal org: NAS stamps NO org_role -> inert, not an error path. + token = _jwt({"sub": "u", "org_id": "org-1"}) + import hermes_cli.auth as auth_mod + monkeypatch.setattr(auth_mod, "resolve_nous_runtime_credentials", + lambda **kw: {"api_key": token, "base_url": "https://x"}) + with pytest.raises(ssc.SyncInertError): + ssc.resolve_org_identity() + assert ssc.org_sync_available() is False + + def test_org_identity_with_role(self, monkeypatch): + token = _jwt({"sub": "u", "org_id": "org-9", "org_role": "MEMBER"}) + import hermes_cli.auth as auth_mod + monkeypatch.setattr(auth_mod, "resolve_nous_runtime_credentials", + lambda **kw: {"api_key": token, "base_url": "https://x"}) + ident = ssc.resolve_org_identity() + assert ident["org_id"] == "org-9" + assert ident["org_role"] == "MEMBER" + assert ssc.org_sync_available() is True + + def test_org_mirror_excluded_from_personal_sync(self, tmp_path, monkeypatch): + # A skill under _org// must never be personal-sync eligible. + skills = tmp_path / "skills" + org_skill = skills / "_org" / "org-1" / "shared-x" + org_skill.mkdir(parents=True) + (org_skill / "SKILL.md").write_text("---\nname: shared-x\n---\n") + monkeypatch.setattr(ssc, "_skills_dir", lambda: skills) + import tools.skill_usage as su + monkeypatch.setattr(su, "is_bundled", lambda n: False) + monkeypatch.setattr(su, "is_hub_installed", lambda n: False) + monkeypatch.setattr(su, "_find_skill_dir", lambda n: org_skill) + import agent.skill_utils as sku + monkeypatch.setattr(sku, "is_external_skill_path", lambda p: False) + assert ssc.is_sync_eligible("shared-x") is False + + +class TestOrgEndToEnd: + def test_admin_propose_merges_directly(self, mock_server, synced_env): + base, state = mock_server + home, skills, identity = synced_env + identity = {**identity, "org_id": "org-1", "org_role": "ADMIN"} + client = ssc.SyncClient(base, identity["api_key"]) + result = ssc.propose_skill("alpha", client, identity=identity) + assert result["ok"] is True + assert result.get("merged") is True + head = state.refs["refs/org/org-1/HEAD"] + assert head == result["head"] + commit = json.loads(state.objects[head][1]) + assert commit["parents"] == [] # first org commit + + def test_member_propose_becomes_202_proposal(self, mock_server, synced_env): + base, state = mock_server + home, skills, identity = synced_env + # Seed an org HEAD as admin first. + admin_ident = {**identity, "org_id": "org-1", "org_role": "ADMIN"} + client = ssc.SyncClient(base, identity["api_key"]) + seeded = ssc.propose_skill("alpha", client, identity=admin_ident) + + # Member edits beta and proposes: server converts to 202. + state.org_role_admin = False + (skills / "devops" / "beta" / "SKILL.md").write_text( + "---\nname: beta\n---\nbeta v2 member edit\n", encoding="utf-8" + ) + member_ident = {**identity, "org_id": "org-1", "org_role": "MEMBER"} + result = ssc.propose_skill("beta", client, identity=member_ident) + assert result["ok"] is True + assert result.get("proposal_pending") is True + assert result["proposal_id"] == 1 + # HEAD untouched; proposal ref parked at the member's commit. + assert state.refs["refs/org/org-1/HEAD"] == seeded["head"] + assert state.refs["refs/org/org-1/proposals/1"] == result["commit"] + # NEVER reported as merged. + assert "merged" not in result + + def test_member_proposal_splices_not_replaces(self, mock_server, synced_env): + # The proposed root must keep the OTHER skills from HEAD (per-skill + # delta, not a wholesale replace). + base, state = mock_server + home, skills, identity = synced_env + admin_ident = {**identity, "org_id": "org-1", "org_role": "ADMIN"} + client = ssc.SyncClient(base, identity["api_key"]) + ssc.propose_skill("alpha", client, identity=admin_ident) + ssc.propose_skill("beta", client, identity=admin_ident) + + state.org_role_admin = False + member_ident = {**identity, "org_id": "org-1", "org_role": "MEMBER"} + result = ssc.propose_skill("alpha", client, identity=member_ident) + # Walk the proposed commit's root: both skills present. + commit = json.loads(state.objects[result["commit"]][1]) + root = json.loads(state.objects[commit["tree"]][1]) + names = {e["name"] for e in root["entries"]} + assert "alpha" in names and "devops" in names + + def test_pull_org_skills_materializes_mirror(self, mock_server, synced_env): + base, state = mock_server + home, skills, identity = synced_env + admin_ident = {**identity, "org_id": "org-1", "org_role": "ADMIN"} + client = ssc.SyncClient(base, identity["api_key"]) + ssc.propose_skill("alpha", client, identity=admin_ident) + + result = ssc.pull_org_skills(client, identity=admin_ident) + assert result["ok"] is True + assert "alpha" in result["updated"] + mirrored = skills / "_org" / "org-1" / "alpha" / "SKILL.md" + assert mirrored.exists() + assert mirrored.read_text().endswith("alpha v1\n") + + def test_pull_org_noop_when_no_head(self, mock_server, synced_env): + base, state = mock_server + home, skills, identity = synced_env + ident = {**identity, "org_id": "org-1", "org_role": "MEMBER"} + client = ssc.SyncClient(base, identity["api_key"]) + result = ssc.pull_org_skills(client, identity=ident) + assert result["ok"] is True + assert result["head"] is None + assert result["updated"] == [] + + def test_propose_requires_org_feature(self, mock_server, synced_env): + base, state = mock_server + home, skills, identity = synced_env + state.org_feature = False + ident = {**identity, "org_id": "org-1", "org_role": "ADMIN"} + client = ssc.SyncClient(base, identity["api_key"]) + with pytest.raises(ssc.SyncInertError): + ssc.propose_skill("alpha", client, identity=ident) + + def test_maybe_pull_org_inert_without_role(self, monkeypatch): + # Personal org: no org_role claim -> None, never raises. + token = _jwt({"sub": "u", "org_id": "org-1"}) + import hermes_cli.auth as auth_mod + monkeypatch.setattr(auth_mod, "resolve_nous_runtime_credentials", + lambda **kw: {"api_key": token}) + assert ssc.maybe_pull_org_skills() is None diff --git a/tools/skill_manager_tool.py b/tools/skill_manager_tool.py index c201ebaffd6e0..9c89e6b5c9412 100644 --- a/tools/skill_manager_tool.py +++ b/tools/skill_manager_tool.py @@ -662,6 +662,81 @@ def _find_skill(name: str) -> Optional[Dict[str, Any]]: return None +def _maybe_auto_propose_org_edit(name: str, skill_path: Path) -> Optional[str]: + """Submit an org-skill edit upstream when `sync.org_auto_propose` is on. + + Returns a short note for the tool result, or None when nothing happened. + Never raises: an offline/failed submission must not fail the edit itself — + the change is already saved locally and can be proposed later. + """ + try: + from agent.skill_utils import is_org_mirror_path + from tools import skills_sync_client as ssc + + if not is_org_mirror_path(skill_path, _skills_dir()): + return None + if not ssc.sync_org_auto_propose(): + return ( + f"This skill is shared by your organisation. Your edit is " + f"saved locally and will not be overwritten by org updates. " + f"Run `hermes sync propose {name}` to share it back." + ) + result = ssc.propose_skill(name) + if result.get("proposal_pending"): + return ( + f"Auto-proposed to your organisation as proposal " + f"#{result.get('proposal_id')} (pending admin review)." + ) + return "Auto-proposed to your organisation (merged into the shared set)." + except Exception as e: + logger.debug("auto-propose skipped for %s: %s", name, e) + return ( + f"Edit saved locally. Could not submit it to your organisation " + f"right now — run `hermes sync propose {name}` to retry." + ) + + +def _org_mirror_write_guard(name: str, skill_path: Path, action: str) -> Optional[Dict[str, Any]]: + """Org-shared skills are EDITABLE IN PLACE — this only blocks deletion. + + Earlier versions refused every write to `_org/`, which broke the learning + loop exactly where it matters most: the agent is told to patch a skill the + moment it finds a gap, and shared skills are the ones the most people use. + Blocking that froze org skills while personal ones kept improving, and the + "fork it into a personal skill" alternative is not something an agent does + mid-task — so improvements were simply lost. + + Now an edit lands in the mirror and is protected from being overwritten by + the next org pull (see the baseline sidecar in skills_sync_client). It + reaches the organisation when the user runs `hermes sync propose`, or + immediately if `sync.org_auto_propose` is on. + + Deletion is still refused: the mirror is a materialized view of the org + HEAD, so a local delete is meaningless (the next pull restores it) and + removing a skill for the organisation is an admin action, not a local one. + """ + if action not in {"delete", "remove_file"}: + return None + try: + from agent.skill_utils import is_org_mirror_path + + if is_org_mirror_path(skill_path, _skills_dir()): + return { + "success": False, + "error": ( + f"Cannot {action} '{name}' locally: it is shared by your " + "organisation, so a local delete would just come back on " + "the next sync. Ask an org admin to remove it for " + "everyone. (Editing it IS allowed — your changes are kept " + "and can be proposed back with `hermes sync propose " + f"{name}`.)" + ), + } + except Exception: + logger.debug("org mirror guard lookup failed for %s", name, exc_info=True) + return None + + def _find_skill_in_other_profiles(name: str) -> List[Tuple[str, Path]]: """Look for ``name`` under SKILL.md across OTHER Hermes profiles. @@ -912,6 +987,9 @@ def _edit_skill(name: str, content: str) -> Dict[str, Any]: existing = _find_skill(name) if not existing: return {"success": False, "error": _skill_not_found_error(name)} + org_guard = _org_mirror_write_guard(name, existing["path"], "edit") + if org_guard: + return org_guard guard = _background_review_write_guard(name, existing["path"], "edit") if guard: return guard @@ -950,6 +1028,10 @@ def _edit_skill(name: str, content: str) -> Dict[str, Any]: "path": str(existing["path"]), "_change": {"description": _desc}, } + org_note = _maybe_auto_propose_org_edit(name, existing["path"]) + if org_note: + result["org_sharing"] = org_note + result["message"] = f"{result['message']} {org_note}" _add_description_prompt_preview(result, content) return result @@ -976,6 +1058,9 @@ def _patch_skill( return {"success": False, "error": _skill_not_found_error(name)} skill_dir = existing["path"] + org_guard = _org_mirror_write_guard(name, skill_dir, "patch") + if org_guard: + return org_guard guard = _background_review_write_guard(name, skill_dir, "patch") if guard: return guard @@ -1064,6 +1149,10 @@ def _patch_skill( "old": old_string[:200] + ("…" if len(old_string) > 200 else ""), "new": new_string[:200] + ("…" if len(new_string) > 200 else ""), } + org_note = _maybe_auto_propose_org_edit(name, skill_dir) + if org_note: + result["org_sharing"] = org_note + result["message"] = f"{result['message']} {org_note}" return result @@ -1082,6 +1171,9 @@ def _delete_skill(name: str, absorbed_into: Optional[str] = None) -> Dict[str, A existing = _find_skill(name) if not existing: return {"success": False, "error": _skill_not_found_error(name)} + org_guard = _org_mirror_write_guard(name, existing["path"], "delete") + if org_guard: + return org_guard guard = _background_review_write_guard(name, existing["path"], "delete") if guard: return guard @@ -1199,6 +1291,9 @@ def _write_file(name: str, file_path: str, file_content: str) -> Dict[str, Any]: existing = _find_skill(name) if not existing: return {"success": False, "error": _skill_not_found_error(name, " Create it first with action='create'.")} + org_guard = _org_mirror_write_guard(name, existing["path"], "write_file") + if org_guard: + return org_guard guard = _background_review_write_guard(name, existing["path"], "write_file") if guard: return guard @@ -1227,11 +1322,16 @@ def _write_file(name: str, file_path: str, file_content: str) -> Dict[str, Any]: target.unlink(missing_ok=True) return {"success": False, "error": scan_error} - return { + result = { "success": True, "message": f"File '{file_path}' written to skill '{name}'.", "path": str(target), } + org_note = _maybe_auto_propose_org_edit(name, existing["path"]) + if org_note: + result["org_sharing"] = org_note + result["message"] = f"{result['message']} {org_note}" + return result def _remove_file(name: str, file_path: str) -> Dict[str, Any]: @@ -1360,6 +1460,56 @@ def apply_skill_pending(payload: Dict[str, Any]) -> str: _skill_gate_bypass.reset(token) +# Debounce state for the sync push hook. A burst of skill_manage writes +# (e.g. create + several write_file calls) collapses into a single push after +# a short quiet window, on a daemon timer so the agent write never blocks. +_sync_push_timer = None +_sync_push_lock = None +_SYNC_PUSH_DEBOUNCE_S = 5.0 + + +def _maybe_debounced_sync_push(skill_name: str) -> None: + """Schedule a debounced best-effort sync push after a skill write. + + Cheap fast-path: if the skill isn't opted into sync, do nothing (no auth, + no network). Otherwise (re)arm a daemon timer; the actual push runs through + ``skills_sync_client.maybe_push_skills`` which enforces the access gate + and swallows all errors. Never blocks the caller (M1-C: agent never blocks + on sync). + """ + global _sync_push_timer, _sync_push_lock + try: + from tools.skill_usage import is_sync_enabled + + if not is_sync_enabled(skill_name): + return + except Exception: + return + + import threading + + if _sync_push_lock is None: + _sync_push_lock = threading.Lock() + + def _fire(): + try: + from tools.skills_sync_client import maybe_push_skills + + maybe_push_skills(message=f"sync: {skill_name}") + except Exception: + pass + + with _sync_push_lock: + if _sync_push_timer is not None: + try: + _sync_push_timer.cancel() + except Exception: + pass + _sync_push_timer = threading.Timer(_SYNC_PUSH_DEBOUNCE_S, _fire) + _sync_push_timer.daemon = True + _sync_push_timer.start() + + def skill_manage( action: str, name: str, @@ -1458,6 +1608,18 @@ def skill_manage( except Exception: pass + # Sync push hook (debounced, best-effort). Fires only AFTER the + # write gate passed (staged/unapproved writes never reach here -- the + # gate returns early above), so we never push un-reviewed content. + # Inert unless the access gate is open (the user is a Nous admin on the + # token), a sync base URL is configured, and the skill is opted into + # sync. Debounced so a burst of edits collapses to one push. Never + # raises -- an agent write must never block on sync (M1-C invariant). + try: + _maybe_debounced_sync_push(name) + except Exception: + pass + return json.dumps(result, ensure_ascii=False) diff --git a/tools/skill_usage.py b/tools/skill_usage.py index f72894374c722..3cfb94856f6cd 100644 --- a/tools/skill_usage.py +++ b/tools/skill_usage.py @@ -458,6 +458,10 @@ def is_curation_eligible(skill_name: str, skill_path: Optional[Path] = None) -> Agent-created skills are always eligible. Bundled built-ins become eligible only when ``curator.prune_builtins`` is enabled. Hub-installed and external skill-dir skills are NEVER eligible — they have an external upstream owner. + Org-shared skills ARE eligible for improvement (the curator may patch them + like any other skill; edits stay local until proposed) but are protected + from ARCHIVE/DELETE elsewhere — removing a shared skill is an org-admin + action, not a local curation decision. Protected built-ins (``PROTECTED_BUILTIN_SKILLS``) are NEVER eligible regardless of any flag — they back load-bearing UX and must never be archived or consolidated. @@ -831,6 +835,26 @@ def set_pinned(skill_name: str, pinned: bool) -> None: _mutate(skill_name, _apply, require_curation_eligible=True) +def set_sync(skill_name: str, sync: bool) -> None: + """Set the sync opt-in flag on a skill's usage record. + + Sync is OPT-IN: nothing propagates to the sync plane unless the user marks + a skill with ``sync: true`` here. Sits alongside ``pinned``/``created_by`` + on the ``.usage.json`` sidecar and is read by + ``tools.skills_sync_client.list_synced_skill_names``. Gated on curation + eligibility so bundled/hub/external skills (which never sync) can't be + marked. Provisional per the M1-D default. + """ + def _apply(rec: Dict[str, Any]) -> None: + rec["sync"] = bool(sync) + _mutate(skill_name, _apply, require_curation_eligible=True) + + +def is_sync_enabled(skill_name: str) -> bool: + """Whether a skill is opted into sync (``sync: true`` in its record).""" + return get_record(skill_name).get("sync") is True + + def forget(skill_name: str) -> None: """Drop a skill's usage entry entirely. Called when the skill is deleted.""" if not skill_name: @@ -989,14 +1013,16 @@ def _find_skill_dir(skill_name: str) -> Optional[Path]: """Locate the directory for a skill by its frontmatter `name:` field. Handles both flat (~/.hermes/skills//SKILL.md) and category-nested - (~/.hermes/skills///SKILL.md) layouts. + (~/.hermes/skills///SKILL.md) layouts. Uses the gated + index iterator so M2 org mirrors resolve ONLY for the active org + (stale ``_org//`` trees never match). """ base = _skills_dir() if not base.exists(): return None - for skill_md in base.rglob("SKILL.md"): - if is_excluded_skill_path(skill_md): - continue + from agent.skill_utils import iter_skill_index_files + + for skill_md in iter_skill_index_files(base, "SKILL.md"): if is_external_skill_path(skill_md): continue if _read_skill_name(skill_md, fallback=skill_md.parent.name) == skill_name: diff --git a/tools/skills_sync_client.py b/tools/skills_sync_client.py new file mode 100644 index 0000000000000..90fe360e9814f --- /dev/null +++ b/tools/skills_sync_client.py @@ -0,0 +1,2093 @@ +#!/usr/bin/env python3 +""" +Skill Sync client -- the low-level sync layer. + +This is the LOW-LEVEL sync layer. It builds content-addressed objects +(blob/tree/commit) from local skills, talks the sync wire contract to a sync +plane (push objects + CAS a ref, pull the owner's HEAD, three-way merge on a +409), and is driven by: + + * a debounced push hook in ``skill_manage`` (after the write-gate passes), + * a periodic pull hook (``maybe_pull_skills``) at the curator tick sites, + * the ``hermes sync status|pull|push|now`` CLI. + +It lives beside ``tools/skills_sync.py`` (NOT under ``hermes_cli/``) so the +low-level sync layer never imports the CLI -- same rule the bundled-skills +sync module documents at ``skills_sync.py:43-50``. + +Contract: the Skill Sync wire contract (version 1, frozen +for Milestone 1). Endpoint shapes, object model, canonicalization, and status +codes below all trace to that document. + +--- ACCESS GATE (pre-launch) --------------------------------------------- +Client sync is INERT (no push, no pull, no-op) unless the signed-in user is a +**Nous admin**. We read that off the access token, which rides on the same +bearer ``resolve_nous_runtime_credentials()`` returns; we decode the JWT +payload (no signature verification -- the server re-verifies) and check the +claim before doing any sync work. + +NAMING: the claim on the wire is ``tool_gateway_admin``, which is misleading +-- it is NOT a tool-gateway-specific right. NAS populates it from +``Permissions.ADMIN_ACCESS`` (access-token-issuer.ts), the same global portal +admin permission that guards ``/admin/*``; the claim is simply named for its +first consumer. We keep the wire name (other services read it) but call it +what it means everywhere on this side. + +This gate is pre-launch containment, not the shipping entitlement. Admin +status conflates "may administer Nous" with "has Skill Sync enabled", and has +no middle setting for a beta cohort -- opening it up would mean handing out +portal admin. Replace it with a real entitlement (a ``sync:*`` scope, a tier +check, or a per-cohort feature flag) before shipping to users. + +--- OPT-IN DEFAULT (M1-D, provisional) ----------------------------------- +Nothing syncs unless the user marks a skill for sync. The user's local intent +is toggled via ``hermes sync enable/disable`` (a ``sync`` flag on the skill's +``.usage.json`` sidecar, alongside ``pinned``/``created_by``), but the DURABLE, +CROSS-DEVICE opt-in state is a committed ``sync-manifest`` object in the sync +plane (design.md §2.8): a root-level blob in the tree at +``refs/user//HEAD`` recording per-skill ``{name, enabled}``. Push writes +the manifest from local intent; pull reconciles local intent FROM it, so a skill +opted in on one device becomes opted in on the others. The plane manifest is +authoritative; the local flag is just the editable intent. Only agent-created + +user-authored skills under ``~/.hermes/skills/`` are eligible; bundled and +hub-installed skills are excluded. +""" + +from __future__ import annotations + +import hashlib +import json +import logging +import os +import time +import stat as _stat +from datetime import datetime, timezone +from pathlib import Path, PurePosixPath +from typing import Any, Callable, Dict, List, Optional, Tuple + +logger = logging.getLogger(__name__) + +# Sync protocol constants +# Wire protocol version. The over-the-wire names below (the `hsp_version` +# capability field and the `x-hsp-object-type` response header) are part of +# the deployed server contract and are NOT renamed with the product — the +# user-facing feature is "Skill Sync"; these are protocol identifiers. +WIRE_VERSION = "1" +DEFAULT_MAX_OBJECT_BYTES = 26214400 # 25 MiB, mirrors capabilities default + +# Object kinds (sync contract) +KIND_BLOB = "blob" +KIND_TREE = "tree" +KIND_COMMIT = "commit" + +# Tree entry modes (sync contract) +MODE_FILE = "file" +MODE_EXEC = "exec" +MODE_DIR = "dir" + +ARTIFACT_TYPE_SKILL = "skill" + +# --------------------------------------------------------------------------- +# `sync-manifest` object convention (design notes). +# +# Per-skill sync opt-in ("this skill syncs / this one does not" +# opt-in state) is CONTENT inside the sync object model, NOT a device-local flag +# or a mutable preference table. An owner's synced set is a small committed blob +# named ``sync-manifest`` at the ROOT of the tree referenced by +# ``refs/user//HEAD``, recording per-skill ``{name, enabled}``. Toggling +# opt-in is a plain CAS ref update (upload the new manifest blob + root tree + +# commit, then CAS HEAD) — the same primitives push already uses. +# +# This makes opt-in durable and CROSS-DEVICE: device B learns which skills the +# user opted in on device A by reading the manifest on pull, rather than each +# device keeping its own local flag. The ``.usage.json`` ``sync`` flag is kept +# only as the local *intent* the user toggles via ``hermes sync enable`` — it is +# reconciled TO the manifest on pull and FROM it on push; the manifest in the +# plane is authoritative. +# +# MUST match gateway-gateway ``src/sync/manifest.ts`` byte-for-byte (the server +# reads + validates this exact shape). Entry name, ``type`` marker, ``version``, +# and the ``{name, enabled}`` skill shape are the shared contract. +# --------------------------------------------------------------------------- + +SYNC_MANIFEST_ENTRY_NAME = "sync-manifest" +SYNC_MANIFEST_TYPE = "sync-manifest" +SYNC_MANIFEST_VERSION = 1 + + +def build_sync_manifest_bytes(skills: Dict[str, bool]) -> bytes: + """Serialize the per-skill opt-in map into canonical ``sync-manifest`` bytes. + + ``skills`` maps skill name -> enabled. Emits the shape gateway-gateway's + ``parseSyncManifest`` validates: ``{type, version:1, skills:[{name,enabled}]}``. + Skill entries are sorted by name for a stable content address. + """ + manifest = { + "type": SYNC_MANIFEST_TYPE, + "version": SYNC_MANIFEST_VERSION, + "skills": [ + {"name": name, "enabled": bool(enabled)} + for name, enabled in sorted(skills.items()) + ], + } + return canonical_json_bytes(manifest) + + +def parse_sync_manifest(data: bytes) -> Optional[Dict[str, bool]]: + """Parse ``sync-manifest`` bytes into ``{name: enabled}``, or ``None`` if the + bytes are not a well-formed manifest. + + Strict (mirrors gateway-gateway ``parseSyncManifest``): an unknown ``type``, + a missing/!=1 ``version``, a non-array ``skills``, or a malformed skill entry + all reject rather than being coerced — a malformed manifest must not be + mistaken for "no skills opted in." + """ + try: + value = json.loads(data.decode("utf-8")) + except Exception: + return None + if not isinstance(value, dict): + return None + if value.get("type") != SYNC_MANIFEST_TYPE: + return None + if value.get("version") != SYNC_MANIFEST_VERSION: + return None + raw_skills = value.get("skills") + if not isinstance(raw_skills, list): + return None + out: Dict[str, bool] = {} + for raw in raw_skills: + if not isinstance(raw, dict): + return None + name = raw.get("name") + enabled = raw.get("enabled") + if not isinstance(name, str) or not name: + return None + if not isinstance(enabled, bool): + return None + out[name] = enabled + return out + + +# --------------------------------------------------------------------------- +# Content addressing +# +# The wire uses the FULL 64-hex sha256 digest. This is a DIFFERENT +# namespace from hermes-agent's local ``content_hash`` (skills_guard.py:846), +# which is a truncated 16-hex digest used for local dedup. They must never be +# conflated -- we compute full digests here. +# --------------------------------------------------------------------------- + +def wire_address(data: bytes) -> str: + """Return ``sha256:<64-hex>`` -- the wire address of ``data``.""" + return "sha256:" + hashlib.sha256(data).hexdigest() + + +def canonical_json_bytes(obj: Dict[str, Any]) -> bytes: + """Canonical JSON serialization for tree/commit hashing (sync contract). + + UTF-8, keys sorted lexicographically, no insignificant whitespace + (``separators=(",", ":")``), no trailing newline. Arrays must already be + in the contract-specified order by the caller (tree entries by ``name``, + commit ``parents`` in significance order). Both client and server MUST + produce byte-identical output or a push fails ``422 hash_mismatch``. + """ + return json.dumps( + obj, + sort_keys=True, + separators=(",", ":"), + ensure_ascii=False, + ).encode("utf-8") + + +# --------------------------------------------------------------------------- +# Identity & access gate +# +# We reuse resolve_nous_runtime_credentials() for the bearer (it honors the +# cross-process file lock + portal host allowlist and refreshes as needed -- +# we do NOT reimplement refresh). The returned api_key IS the JWT bearer; we +# decode its payload (unverified) to read the access-gate claim. +# --------------------------------------------------------------------------- + +# Dev-phase gate claim (NAS access-token-issuer.ts:312). Sync is inert unless +# the resolved token carries this claim === true. Remove when sync ships GA. +# Wire claim name is NAS's; it means "this user is a Nous admin" +# (populated from Permissions.ADMIN_ACCESS), NOT a tool-gateway right. +NOUS_ADMIN_CLAIM = "tool_gateway_admin" + + +class SyncInertError(RuntimeError): + """Raised (and caught by the gate-and-swallow hooks) when sync must no-op: + + not logged in, no bearer, or the caller is not a Nous admin. + """ + + +def _decode_jwt_payload_unverified(token: str) -> Dict[str, Any]: + """Decode a JWT payload WITHOUT signature verification. + + Safe here: we never trust these claims for authz -- the server re-verifies + every call. We only read the dev-gate claim to decide whether to attempt + sync at all. Mirrors the diagnostic decode in + plugins/dashboard_auth/nous/__init__.py:463. + """ + try: + import jwt # PyJWT, a core dependency + + return jwt.decode( + token, + options={"verify_signature": False, "verify_exp": False}, + ) or {} + except Exception as e: + logger.debug("skills_sync_client: JWT payload decode failed: %s", e) + return {} + + +def resolve_identity() -> Dict[str, Any]: + """Resolve the Nous bearer + owner + dev-gate flag. + + Returns a dict: ``{api_key, base_url, owner, nous_admin, claims}``. + Raises :class:`SyncInertError` if not logged in / no bearer. + + ``owner`` is the token-verified subject; the server derives the real owner + from the bearer regardless (contract §0.4), so this is advisory for local + ref naming only. + """ + try: + from hermes_cli.auth import resolve_nous_runtime_credentials + + creds = resolve_nous_runtime_credentials() + except Exception as e: + raise SyncInertError(f"no Nous credentials: {e}") from e + + api_key = (creds or {}).get("api_key") + if not api_key: + raise SyncInertError("no bearer token available") + + claims = _decode_jwt_payload_unverified(api_key) + owner = ( + claims.get("sub") + or claims.get("privy_did") + or claims.get("tid") + or "unknown" + ) + nous_admin = claims.get(NOUS_ADMIN_CLAIM) is True + return { + "api_key": api_key, + "base_url": (creds or {}).get("base_url"), + "owner": str(owner), + "nous_admin": nous_admin, + "claims": claims, + } + + +def dev_gate_open() -> bool: + """Whether the access gate permits sync. Never raises.""" + try: + return bool(resolve_identity().get("nous_admin")) + except SyncInertError: + return False + except Exception as e: + logger.debug("skills_sync_client: dev_gate_open check failed: %s", e) + return False + + +# --------------------------------------------------------------------------- +# Sync-plane endpoint resolution +# +# The sync routes are mounted under /v1/sync/. The base URL defaults to the +# production plane, so a normal user configures nothing; config.yaml +# sync.base_url (or the HERMES_SYNC_BASE_URL bridge env) overrides it to point +# a dev/staging build at another plane. It is NOT the inference base_url. +# --------------------------------------------------------------------------- + +#: Production Skill Sync plane. Overridable per the resolution order below. +DEFAULT_SYNC_BASE_URL = "https://gateway-gateway.nousresearch.com" + +def resolve_sync_base_url() -> Optional[str]: + """Resolve the sync-plane base URL. + + Order: HERMES_SYNC_BASE_URL env bridge -> config.yaml ``sync.base_url`` -> + the production plane. Returns a base without a trailing slash (e.g. + ``https://host``); the ``/v1/sync/`` prefix is appended by the client. + + The production default means a normal user never configures a URL — the + env var and config key exist to point a dev/staging build at another + plane. Returns None only if the default is somehow blanked out. + """ + env = os.getenv("HERMES_SYNC_BASE_URL") + if env and env.strip(): + return env.strip().rstrip("/") + try: + # Lazy import: the low-level sync layer must not import the CLI at + # module load (skills_sync.py:43-50). A function-scoped import avoids + # the cycle -- same pattern agent/curator.py:141 uses for config. + from hermes_cli.config import load_config + + cfg = load_config() or {} + sync_cfg = cfg.get("sync") or {} + base = sync_cfg.get("base_url") + if isinstance(base, str) and base.strip(): + return base.strip().rstrip("/") + except Exception as e: + logger.debug("skills_sync_client: config sync.base_url read failed: %s", e) + return DEFAULT_SYNC_BASE_URL or None + + +# --------------------------------------------------------------------------- +# Sync feature configuration — env-first, so a Hermes Cloud instance can be set +# up to use sync BY DEFAULT purely through environment variables (no per-user +# config.yaml edit, no per-skill CLI call). Every knob follows the same +# precedence as base_url: the HERMES_SYNC_* env var wins, else config.yaml +# ``sync.*``, else a built-in default. +# +# HERMES_SYNC_BASE_URL -> sync.base_url (the sync plane URL) +# HERMES_SYNC_ENABLED -> sync.enabled (master on/off; default off) +# HERMES_SYNC_DEFAULT_OPT_IN -> sync.default_opt_in (personal sync policy; default false +# = opt-in. Set true to make +# every eligible skill sync +# without per-skill enable — +# the opt-OUT default a Cloud +# deployment wants.) +# --------------------------------------------------------------------------- + +_TRUE = {"1", "true", "yes", "on"} +_FALSE = {"0", "false", "no", "off", ""} + + +def _parse_bool(value: Any) -> Optional[bool]: + """Parse a config/env bool. Returns None if unrecognized (so callers can + fall through to the next precedence layer). Accepts real bools + strings.""" + if isinstance(value, bool): + return value + if value is None: + return None + s = str(value).strip().lower() + if s in _TRUE: + return True + if s in _FALSE: + return False + return None + + +def _sync_config_bool(env_var: str, config_key: str, *, default: bool) -> bool: + """Resolve a boolean sync knob: ``env_var`` -> ``sync.`` -> default.""" + env_val = _parse_bool(os.getenv(env_var)) + if env_val is not None: + return env_val + try: + from hermes_cli.config import load_config + + cfg = load_config() or {} + sync_cfg = cfg.get("sync") or {} + cfg_val = _parse_bool(sync_cfg.get(config_key)) + if cfg_val is not None: + return cfg_val + except Exception as e: + logger.debug("skills_sync_client: config sync.%s read failed: %s", config_key, e) + return default + + +def sync_feature_enabled() -> bool: + """Whether the sync feature is turned on for this instance (env-first). + + ``HERMES_SYNC_ENABLED`` -> ``sync.enabled`` -> False. This is the master + switch a Hermes Cloud deployment sets to opt its instances into sync by + default. It is checked by the gate-and-swallow entrypoints IN ADDITION to + the Nous-admin token gate and a configured base URL — all three must hold for + background sync to run. + """ + return _sync_config_bool("HERMES_SYNC_ENABLED", "enabled", default=False) + + +def sync_org_auto_propose() -> bool: + """Whether an agent/user edit to an org skill is proposed automatically. + + ``HERMES_SYNC_ORG_AUTO_PROPOSE`` -> ``sync.org_auto_propose`` -> False. + + False (default): edits to an org-shared skill stay LOCAL until the user + runs ``hermes sync propose ``. The skill keeps working with the + edit applied; the organisation just doesn't see it yet. + + True: every local edit to an org skill is submitted to the org as a + proposal right away (an admin still approves it, unless the editor is an + admin). Suits a small, high-trust team that wants improvements to flow + back without anyone remembering to push them. + """ + return _sync_config_bool( + "HERMES_SYNC_ORG_AUTO_PROPOSE", "org_auto_propose", default=False + ) + + +def sync_default_opt_in() -> bool: + """The personal sync default opt-in policy (env-first). + + ``HERMES_SYNC_DEFAULT_OPT_IN`` -> ``sync.default_opt_in`` -> False. + + False (default): opt-IN — a skill syncs only after an explicit + ``hermes sync enable`` (or a plane manifest that opted it in). True: opt-OUT + — every sync-eligible skill is treated as opted in unless explicitly + disabled, which is the "your skills follow you with no setup" default a + Hermes Cloud deployment wants. Per the design notes, this default is + provisional and expected to flip; exposing it as env config lets the + operator choose per deployment without a protocol change. + """ + return _sync_config_bool("HERMES_SYNC_DEFAULT_OPT_IN", "default_opt_in", default=False) + + +# --------------------------------------------------------------------------- +# Local skill eligibility + the personal sync opt-in "sync" flag +# +# Only agent-created + user-authored skills under ~/.hermes/skills/ sync. +# Bundled (.bundled_manifest) and hub-installed skills are excluded. Sync is +# opt-in: a skill only syncs when its usage-sidecar carries ``sync: true``. +# --------------------------------------------------------------------------- + +def _skills_dir() -> Path: + from hermes_constants import get_hermes_home + + return get_hermes_home() / "skills" + + +def is_sync_eligible(skill_name: str) -> bool: + """Whether *skill_name* is a candidate for sync (before the opt-in check). + + Eligible = present locally under ~/.hermes/skills/, NOT bundled, NOT + hub-installed, NOT an external-dir skill, and NOT under the org mirror + (``_org/`` — enterprise-managed content pulls from the org HEAD and must + never ride a personal push; the sync contract / the design notes). Mirrors the + exclusion logic used by the curator (tools/skill_usage.py). + """ + try: + from tools.skill_usage import is_bundled, is_hub_installed, _find_skill_dir + from agent.skill_utils import is_external_skill_path + except Exception: + return False + if is_bundled(skill_name) or is_hub_installed(skill_name): + return False + skill_dir = _find_skill_dir(skill_name) + if skill_dir is None: + return False + if is_external_skill_path(skill_dir): + return False + try: + rel = skill_dir.resolve().relative_to(_skills_dir().resolve()) + if rel.parts and rel.parts[0] == ORG_DIR_NAME: + return False + except (OSError, ValueError): + pass + return True + + +def list_synced_skill_names() -> List[str]: + """Return the names of skills that should sync, honoring the opt-in policy. + + Two policies (``sync_default_opt_in()``, env-first — see that function): + + - **opt-in (default):** a skill syncs only when its usage record carries + ``sync: true`` AND it is eligible. Nothing syncs by default. + - **opt-out (Hermes Cloud "on by default"):** every *eligible* skill syncs + UNLESS its usage record explicitly carries ``sync: false``. This is what a + deployment sets (via ``HERMES_SYNC_DEFAULT_OPT_IN``) so a user's skills + follow them with no per-skill setup. + + Sorted, deduped. + """ + try: + from tools.skill_usage import load_usage + except Exception: + return [] + usage = load_usage() or {} + + if sync_default_opt_in(): + # opt-OUT: all eligible skills except those explicitly turned off. + names = [] + for name in _all_local_skill_names(): + rec = usage.get(name) + if isinstance(rec, dict) and rec.get("sync") is False: + continue # explicit opt-out wins over the deployment default + if is_sync_eligible(name): + names.append(name) + return sorted(set(names)) + + # opt-IN (default): only explicitly-enabled eligible skills. + names = [] + for name, rec in usage.items(): + if isinstance(rec, dict) and rec.get("sync") is True and is_sync_eligible(name): + names.append(name) + return sorted(set(names)) + + +def _all_local_skill_names() -> List[str]: + """Best-effort enumeration of every locally-present skill name (used by the + opt-out policy). A skill is any directory under ~/.hermes/skills/ containing + a ``SKILL.md``; the name is its frontmatter ``name`` (falling back to the + directory name). Eligibility (bundled/hub/external exclusion) is applied by + the caller via ``is_sync_eligible``. + """ + names: List[str] = [] + root = _skills_dir() + try: + if not root.exists(): + return [] + for skill_md in root.rglob("SKILL.md"): + if skill_md.is_symlink(): + continue + name: Optional[str] = None + try: + from tools.skill_usage import _read_skill_name + + name = _read_skill_name(skill_md, skill_md.parent.name) + except Exception: + name = skill_md.parent.name + if name: + names.append(name) + except OSError as e: + logger.debug("skills_sync_client: local skill enumeration failed: %s", e) + return sorted(set(names)) + + +# --------------------------------------------------------------------------- +# Object building -- turn a skill directory into blob/tree/commit objects +# +# A skill dir becomes one tree (sync contract). Each file is a blob; each +# subdir a nested tree. The profile-root tree (the sync contract: "a tree whose +# entries are category trees") is built from the set of synced skill trees. +# --------------------------------------------------------------------------- + +class ObjectSet: + """Accumulates objects to push: hash -> (kind, bytes). + + Deduped by content address, so identical blobs across skills upload once. + """ + + def __init__(self) -> None: + self.objects: Dict[str, Tuple[str, bytes]] = {} + + def add(self, kind: str, data: bytes) -> str: + addr = wire_address(data) + self.objects.setdefault(addr, (kind, data)) + return addr + + def __len__(self) -> int: + return len(self.objects) + + +def _file_mode(path: Path) -> str: + """Return the tree mode for a regular file: ``exec`` if +x else ``file`` + (contract §2.3). No symlinks / other modes are emitted.""" + try: + if path.stat().st_mode & (_stat.S_IXUSR | _stat.S_IXGRP | _stat.S_IXOTH): + return MODE_EXEC + except OSError: + pass + return MODE_FILE + + +def build_tree(dir_path: Path, objects: ObjectSet, *, max_object_bytes: int) -> str: + """Recursively build objects for *dir_path*; return the tree address. + + Regular files become blobs; subdirectories become nested trees. Symlinks, + sockets, and other special files are skipped (contract §2.3 security: no + symlinks). Blobs over *max_object_bytes* raise :class:`ValueError` so the + caller can surface / skip the artifact (contract §4.3 -> 413). + """ + entries: List[Dict[str, str]] = [] + for child in sorted(dir_path.iterdir(), key=lambda p: p.name): + if child.is_symlink(): + logger.debug("skills_sync_client: skipping symlink %s", child) + continue + if child.is_dir(): + sub_hash = build_tree(child, objects, max_object_bytes=max_object_bytes) + entries.append( + {"name": child.name, "kind": KIND_TREE, "hash": sub_hash, "mode": MODE_DIR} + ) + elif child.is_file(): + data = child.read_bytes() + if len(data) > max_object_bytes: + raise ValueError( + f"file {child} is {len(data)} bytes > max_object_bytes " + f"{max_object_bytes}" + ) + blob_hash = objects.add(KIND_BLOB, data) + entries.append( + { + "name": child.name, + "kind": KIND_BLOB, + "hash": blob_hash, + "mode": _file_mode(child), + } + ) + # else: skip special files + # Entries sorted by name (byte order) for canonicalization (sync contract). + entries.sort(key=lambda e: e["name"]) + tree_obj = {"type": KIND_TREE, "entries": entries} + return objects.add(KIND_TREE, canonical_json_bytes(tree_obj)) + + +def build_commit( + tree_hash: str, + parents: List[str], + *, + owner: str, + device: str, + message: str, + objects: ObjectSet, + ts: Optional[str] = None, +) -> str: + """Build a commit object (sync contract) and return its address. + + ``parents``: 0 for first commit, 1 for a normal edit, 2 for a merge commit + (order significant: parents[0] = base fast-forwarded from, parents[1] = + the other head being merged). + """ + commit_obj = { + "type": KIND_COMMIT, + "tree": tree_hash, + "parents": list(parents), + "author": {"owner": owner, "device": device}, + "ts": ts or datetime.now(timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ"), + "message": message, + "artifact_type": ARTIFACT_TYPE_SKILL, + } + return objects.add(KIND_COMMIT, canonical_json_bytes(commit_obj)) + + +def _default_device_label() -> str: + """A human-friendly default device label: the short hostname plus a short + random suffix for uniqueness (two machines can share a hostname). Falls back + to a bare uuid if the hostname is unavailable/unusable.""" + import socket + import uuid + + suffix = uuid.uuid4().hex[:6] + try: + host = socket.gethostname() or "" + except OSError: + host = "" + # Short hostname (drop domain), strip to a tidy slug; keep it readable. + short = host.split(".")[0].strip() + # Keep only sane chars so the label renders cleanly in the console. + short = "".join(c for c in short if c.isalnum() or c in "-_") or "" + return f"{short}-{suffix}" if short else uuid.uuid4().hex + + +def stable_device_id() -> str: + """Return a stable per-device label for commit ``author.device`` (contract + -- advisory, never an auth input). Persisted under + ~/.hermes/skills/.sync_device_id. + + New devices are seeded with a HUMAN-FRIENDLY default (short hostname + a + short random suffix, e.g. ``bens-macbook-a1b2c3``) so the sync console shows + something recognizable instead of an opaque hash. Existing ``.sync_device_id`` + files are honored verbatim (backward-compatible — a machine keeps its id). + Use ``set_device_name()`` / ``hermes sync device --name`` to set an explicit + label.""" + path = _skills_dir() / ".sync_device_id" + try: + if path.exists(): + val = path.read_text(encoding="utf-8").strip() + if val: + return val + except OSError: + pass + + # Hermes Cloud (and any templated deployment) can seed the label + # declaratively via HERMES_SYNC_DEVICE_NAME, so a hosted instance shows a + # recognizable name with no CLI call. Env seeds the FIRST-USE value only; it + # is then persisted, so a later `hermes sync device --name` (or editing the + # file) still wins on that device. An explicit file (above) always wins over + # the env. + import os + + env_name = (os.environ.get("HERMES_SYNC_DEVICE_NAME") or "").strip() + val = env_name if env_name else _default_device_label() + try: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(val, encoding="utf-8") + except OSError as e: + logger.debug("skills_sync_client: could not persist device id: %s", e) + return val + + +def set_device_name(name: str) -> str: + """Set the human-friendly device label used for commit ``author.device``. + + Writes the (trimmed) name to ~/.hermes/skills/.sync_device_id, overwriting + any previous value. The label is advisory metadata only — never an auth + input (contract §2.4) — so any non-empty string is accepted. Returns the + stored value. Raises ValueError on an empty name. + """ + cleaned = (name or "").strip() + if not cleaned: + raise ValueError("device name must be a non-empty string") + path = _skills_dir() / ".sync_device_id" + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(cleaned, encoding="utf-8") + return cleaned + + +# --------------------------------------------------------------------------- +# Sync wire client +# +# Thin requests-based client for the endpoints in the sync contract- Uploads all +# new objects (batch), then CAS-es the ref. A 409 returns the actual head for +# the caller's three-way merge. Auth is the Nous bearer resolved above. +# --------------------------------------------------------------------------- + +class SyncError(RuntimeError): + """A non-recoverable wire error (4xx that the client can't retry).""" + + def __init__(self, message: str, *, status: Optional[int] = None): + super().__init__(message) + self.status = status + + +class SyncConflict(RuntimeError): + """CAS lost (409). ``actual`` is the current head to merge against + (contract §4.4). NOT a rejection -- pushed objects are already durable.""" + + def __init__(self, actual: str): + super().__init__(f"CAS conflict; actual head {actual}") + self.actual = actual + + +class SyncClient: + """Sync client bound to a base URL + bearer (routes under + ``/v1/sync/``).""" + + def __init__(self, base_url: str, api_key: str, *, timeout: float = 30.0): + self.base = base_url.rstrip("/") + self.api_key = api_key + self.timeout = timeout + import requests # core dependency + + self._session = requests.Session() + self._session.headers["Authorization"] = f"Bearer {api_key}" + + def _url(self, path: str) -> str: + return f"{self.base}/v1/sync/{path.lstrip('/')}" + + # -- capability & read ------------------------------------------------- + + def capabilities(self) -> Dict[str, Any]: + """GET /v1/sync/capabilities (sync contract). No auth required.""" + r = self._session.get(self._url("capabilities"), timeout=self.timeout) + if r.status_code != 200: + raise SyncError(f"capabilities failed: {r.status_code}", status=r.status_code) + return r.json() + + def get_refs(self, prefix: str) -> List[Dict[str, str]]: + """GET /v1/sync/refs?prefix=... (sync contract).""" + r = self._session.get( + self._url("refs"), params={"prefix": prefix}, timeout=self.timeout + ) + if r.status_code != 200: + raise SyncError(f"get_refs failed: {r.status_code}", status=r.status_code) + return (r.json() or {}).get("refs", []) + + def get_object(self, obj_hash: str) -> Tuple[str, bytes]: + """GET /v1/sync/objects/:hash (sync contract). Returns (kind, bytes). + + Kind comes from the object-type response header for tree/commit; a blob + (application/octet-stream) is returned as ``blob``. + """ + r = self._session.get(self._url(f"objects/{obj_hash}"), timeout=self.timeout) + if r.status_code == 404: + raise SyncError(f"object {obj_hash} not found", status=404) + if r.status_code == 403: + raise SyncError(f"object {obj_hash} not readable", status=403) + if r.status_code != 200: + raise SyncError(f"get_object failed: {r.status_code}", status=r.status_code) + kind = r.headers.get("X-HSP-Object-Type") or KIND_BLOB + return kind, r.content + + def get_commit_json(self, commit_hash: str) -> Dict[str, Any]: + """Fetch a commit object and parse its canonical JSON.""" + kind, data = self.get_object(commit_hash) + if kind != KIND_COMMIT: + raise SyncError(f"{commit_hash} is {kind}, expected commit") + return json.loads(data.decode("utf-8")) + + def get_tree_json(self, tree_hash: str) -> Dict[str, Any]: + """Fetch a tree object and parse its canonical JSON.""" + kind, data = self.get_object(tree_hash) + if kind != KIND_TREE: + raise SyncError(f"{tree_hash} is {kind}, expected tree") + return json.loads(data.decode("utf-8")) + + # -- write ------------------------------------------------------------- + + def put_objects( + self, + objects: Dict[str, Tuple[str, bytes]], + *, + org_scope: bool = False, + ) -> Dict[str, Any]: + """POST /v1/sync/objects (sync contract). Batch multi-object upload. + + Contract §1 requires raw object bytes on the wire (NOT base64-in-JSON), + and specifies "a length-prefixed or multipart stream of + {hash, type, bytes}". We use multipart/form-data: one part per object, + the part's field name = the claimed ``sha256:`` hash, its + ``filename`` carries the object ``type`` (blob|tree|commit), and the + part body is the raw object bytes. The server recomputes each hash from + the received bytes and rejects the whole batch with 422 on mismatch. + Idempotent: a known hash is a no-op ``already_present``. + + M2 (contract §11.5): ``org_scope=True`` adds ``?scope=org`` so the + objects land in the ORG scope (org-readable; required before an org + CAS/propose). Gated server-side on the token's org_role claim. + + NOTE (framing choice within contract latitude): §4.2 says "length- + prefixed OR multipart"; this picks multipart/form-data with + (field=hash, filename=type, body=raw-bytes). The server strand must + parse the same framing -- flagged for cross-strand alignment. + """ + # (field_name, (filename, raw_bytes, content_type)) + files = [ + (h, (kind, data, "application/octet-stream")) + for h, (kind, data) in objects.items() + ] + r = self._session.post( + self._url("objects"), + files=files, + params={"scope": "org"} if org_scope else None, + timeout=self.timeout, + ) + if r.status_code == 413: + raise SyncError("object too large (413)", status=413) + if r.status_code == 422: + raise SyncError(f"hash_mismatch (422): {r.text}", status=422) + if r.status_code not in (200, 201): + raise SyncError(f"put_objects failed: {r.status_code}", status=r.status_code) + return r.json() if r.content else {} + + def cas_ref(self, name: str, from_hash: Optional[str], to_hash: str) -> Dict[str, Any]: + """POST /v1/sync/refs/:name -- atomic compare-and-swap (sync contract). + + Raises :class:`SyncConflict` (carrying the actual head) on 409. + + M2 (contract §11.5): a non-admin member's CAS on an org HEAD is never + rejected — the server converts it to a proposal and returns + ``202 {proposal_id, ref}``. Surfaced as + ``{"proposal_pending": True, ...}`` so callers can tell "merged" (200) + from "proposed, awaiting review" (202) without exceptions — a 202 is a + SUCCESS-shaped outcome, never to be presented as live (error table §5). + """ + r = self._session.post( + self._url(f"refs/{name}"), + json={"from": from_hash, "to": to_hash}, + timeout=self.timeout, + ) + if r.status_code == 202: + body = r.json() if r.content else {} + return {"proposal_pending": True, **body} + if r.status_code == 409: + actual = (r.json() or {}).get("actual", "") + raise SyncConflict(actual) + if r.status_code == 403: + raise SyncError("forbidden (403) -- owner/permission", status=403) + if r.status_code != 200: + raise SyncError(f"cas_ref failed: {r.status_code}", status=r.status_code) + return r.json() if r.content else {} + + +# --------------------------------------------------------------------------- +# Local sync STATE (client-local head bookkeeping, FULL-digest namespace) +# +# Records the last commit HEAD we pushed/pulled and, per synced skill, the tree +# hash of the on-disk content at that point. Distinct from the bundled manifest +# (skills_sync.py, truncated local content_hash namespace) AND from the +# `sync-manifest` OBJECT in the sync plane (the per-skill opt-in content). This +# is purely local reconciliation bookkeeping. Lives at +# ~/.hermes/skills/.sync_state as JSON. +# +# NOTE: renamed from `.sync_manifest` -> `.sync_state` to remove the collision +# with the plane `sync-manifest`. `read_sync_state` migrates an existing +# `.sync_manifest` on first read so no local head record is lost. +# --------------------------------------------------------------------------- + +def _sync_state_path() -> Path: + return _skills_dir() / ".sync_state" + + +def _legacy_sync_state_path() -> Path: + return _skills_dir() / ".sync_manifest" + + +def read_sync_state() -> Dict[str, Any]: + """Read the local sync state. Returns a default on missing/corrupt. + + Shape: ``{"head": "sha256:...|null", "skills": {name: {tree, commit}}}``. + ``head`` is the last profile-root HEAD commit we reconciled with. + + Migrates a legacy ``.sync_manifest`` file (pre-rename) transparently: if the + new ``.sync_state`` is absent but the legacy file exists, it is read and + rewritten to the new path so an existing device keeps its head record. + """ + path = _sync_state_path() + if not path.exists(): + legacy = _legacy_sync_state_path() + if legacy.exists(): + try: + data = json.loads(legacy.read_text(encoding="utf-8")) + if isinstance(data, dict): + data.setdefault("head", None) + data.setdefault("skills", {}) + write_sync_state(data) # migrate to the new path + try: + legacy.unlink() + except OSError: + pass + return data + except (OSError, json.JSONDecodeError) as e: + logger.debug("skills_sync_client: legacy sync state migrate failed: %s", e) + return {"head": None, "skills": {}} + try: + data = json.loads(path.read_text(encoding="utf-8")) + if isinstance(data, dict): + data.setdefault("head", None) + data.setdefault("skills", {}) + return data + except (OSError, json.JSONDecodeError) as e: + logger.debug("skills_sync_client: sync state read failed: %s", e) + return {"head": None, "skills": {}} + + +def write_sync_state(data: Dict[str, Any]) -> None: + """Write the local sync state atomically. Best-effort.""" + import tempfile + + path = _sync_state_path() + try: + path.parent.mkdir(parents=True, exist_ok=True) + fd, tmp = tempfile.mkstemp(dir=str(path.parent), prefix=".sync_state_", suffix=".tmp") + try: + with os.fdopen(fd, "w", encoding="utf-8") as f: + json.dump(data, f, indent=2, sort_keys=True, ensure_ascii=False) + f.flush() + os.fsync(f.fileno()) + os.replace(tmp, path) + except BaseException: + try: + os.unlink(tmp) + except OSError: + pass + raise + except Exception as e: + logger.debug("skills_sync_client: sync state write failed: %s", e) + + +# --------------------------------------------------------------------------- +# Tree materialization (pull) -- write a tree back to a skill directory +# --------------------------------------------------------------------------- + +def materialize_tree(client: SyncClient, tree_hash: str, dest: Path) -> None: + """Write the tree at *tree_hash* into *dest* (created if needed). + + Blobs become files (with +x restored for ``exec`` mode), nested trees + become subdirectories. Does NOT delete files absent from the tree -- the + caller decides removal semantics. Refuses path traversal via entry names. + """ + dest.mkdir(parents=True, exist_ok=True) + tree = client.get_tree_json(tree_hash) + for entry in tree.get("entries", []): + name = entry.get("name", "") + if not name or "/" in name or name in (".", ".."): + logger.warning("skills_sync_client: skipping unsafe tree entry %r", name) + continue + target = dest / name + kind = entry.get("kind") + if kind == KIND_TREE: + materialize_tree(client, entry["hash"], target) + elif kind == KIND_BLOB: + _, data = client.get_object(entry["hash"]) + target.write_bytes(data) + if entry.get("mode") == MODE_EXEC: + try: + st = target.stat().st_mode + target.chmod(st | _stat.S_IXUSR | _stat.S_IXGRP | _stat.S_IXOTH) + except OSError: + pass + + +# --------------------------------------------------------------------------- +# Profile snapshot -- build the objects + per-skill tree map for a push +# +# The profile root is a tree whose entries mirror each synced skill's relative +# path under ~/.hermes/skills/ (the sync contract: "the profile root is a tree +# whose entries are category trees"). Only opted-in, eligible skills are +# included (personal sync opt-in + eligibility). +# --------------------------------------------------------------------------- + +def _skill_rel_path(skill_name: str) -> Optional[PurePosixPath]: + """Return the skill's path relative to ~/.hermes/skills/ (posix), or None.""" + try: + from tools.skill_usage import _find_skill_dir + except Exception: + return None + skill_dir = _find_skill_dir(skill_name) + if skill_dir is None: + return None + try: + rel = skill_dir.resolve().relative_to(_skills_dir().resolve()) + except (OSError, ValueError): + return None + return PurePosixPath(rel.as_posix()) + + +def snapshot_profile( + skill_names: List[str], *, max_object_bytes: int = DEFAULT_MAX_OBJECT_BYTES +) -> Tuple[ObjectSet, str, Dict[str, str]]: + """Build all objects for *skill_names* + the profile-root tree. + + Returns ``(objects, root_tree_hash, skill_tree_map)`` where + ``skill_tree_map`` is ``{skill_name: tree_hash}``. Skills whose blobs + exceed *max_object_bytes* are skipped (surfaced via logger). + + The root tree nests category directories: a skill at ``devops/foo`` yields + a root entry ``devops`` (tree) containing ``foo`` (tree). Flat skills yield + a direct root entry. + + The root tree also carries a ``sync-manifest`` BLOB (design.md §2.8) + recording the per-skill opt-in state, so opt-in is durable + cross-device + rather than a device-local ``.usage.json`` flag. Every skill in + ``skill_names`` is recorded ``enabled: true`` (they ARE the opted-in set); + the manifest is the authoritative record the plane + other devices read. + """ + from tools.skill_usage import _find_skill_dir + + objects = ObjectSet() + skill_tree_map: Dict[str, str] = {} + # Nested dict representing the root: {name: {"__tree__": hash} | subdict} + root: Dict[str, Any] = {} + + for name in sorted(set(skill_names)): + rel = _skill_rel_path(name) + skill_dir = _find_skill_dir(name) + if rel is None or skill_dir is None: + continue + try: + tree_hash = build_tree(skill_dir, objects, max_object_bytes=max_object_bytes) + except ValueError as e: + logger.warning("skills_sync_client: skipping %s: %s", name, e) + continue + skill_tree_map[name] = tree_hash + # Insert into the nested root structure by relative path parts. + parts = list(rel.parts) + node = root + for part in parts[:-1]: + node = node.setdefault(part, {}) + node[parts[-1]] = {"__tree__": tree_hash} + + # sync-manifest: record the opt-in state (the pushed set = enabled). + # Only skills that actually made it into the tree are recorded, keyed by the + # skill NAME (matching gateway-gateway's manifest shape + the read walk that + # enumerates skill subtrees by name). + manifest_map = {name: True for name in skill_tree_map} + manifest_hash = objects.add( + KIND_BLOB, build_sync_manifest_bytes(manifest_map) + ) + + root_hash = _build_root_tree(root, objects, manifest_hash=manifest_hash) + return objects, root_hash, skill_tree_map + + +def _build_root_tree( + node: Dict[str, Any], objects: ObjectSet, *, manifest_hash: Optional[str] = None +) -> str: + """Recursively canonicalize the nested root structure into trees. + + ``manifest_hash`` (only passed at the top level) adds a root-level + ``sync-manifest`` BLOB entry (design.md §2.8) alongside the skill subtrees. + It cannot collide with a skill dir (skill entries are trees; this is a blob). + """ + entries: List[Dict[str, str]] = [] + for name, child in node.items(): + if isinstance(child, dict) and "__tree__" in child and len(child) == 1: + entries.append( + {"name": name, "kind": KIND_TREE, "hash": child["__tree__"], "mode": MODE_DIR} + ) + else: + sub_hash = _build_root_tree(child, objects) + entries.append( + {"name": name, "kind": KIND_TREE, "hash": sub_hash, "mode": MODE_DIR} + ) + if manifest_hash is not None: + entries.append( + { + "name": SYNC_MANIFEST_ENTRY_NAME, + "kind": KIND_BLOB, + "hash": manifest_hash, + "mode": MODE_FILE, + } + ) + entries.sort(key=lambda e: e["name"]) + tree_obj = {"type": KIND_TREE, "entries": entries} + return objects.add(KIND_TREE, canonical_json_bytes(tree_obj)) + + +# --------------------------------------------------------------------------- +# Ref naming (sync contract) +# --------------------------------------------------------------------------- + +def user_head_ref(owner: str) -> str: + return f"refs/user/{owner}/HEAD" + + +def user_conflict_ref(owner: str, n: int) -> str: + return f"refs/user/{owner}/conflict/{n}" + + +def _root_tree_of_commit(client: "SyncClient", commit_hash: str) -> str: + """Return the tree hash referenced by a commit.""" + return client.get_commit_json(commit_hash)["tree"] + + +def _skill_trees_of_root(client: "SyncClient", root_tree_hash: str) -> Dict[str, str]: + """Flatten a profile-root tree into ``{posix_rel_path: skill_tree_hash}``. + + A skill tree is any tree containing a ``SKILL.md`` blob entry. We walk the + root tree; a subtree with a SKILL.md is treated as a skill leaf keyed by + its path, so category nesting is preserved. + """ + result: Dict[str, str] = {} + + def _walk(tree_hash: str, prefix: str) -> None: + tree = client.get_tree_json(tree_hash) + entries = tree.get("entries", []) + has_skill_md = any( + e.get("name") == "SKILL.md" and e.get("kind") == KIND_BLOB for e in entries + ) + if has_skill_md and prefix: + result[prefix] = tree_hash + return + for e in entries: + if e.get("kind") == KIND_TREE: + child_prefix = f"{prefix}/{e['name']}" if prefix else e["name"] + _walk(e["hash"], child_prefix) + + _walk(root_tree_hash, "") + return result + + +def read_manifest_of_root( + client: "SyncClient", root_tree_hash: str +) -> Optional[Dict[str, bool]]: + """Read the ``sync-manifest`` blob at the root of *root_tree_hash* into + ``{name: enabled}`` (design.md §2.8), or ``None`` if there is no manifest + entry / it is malformed. + + The manifest is a root-level BLOB entry named ``sync-manifest`` (never a + skill subtree). This is how a device learns the cross-device opt-in state + written by another device's push. + """ + try: + tree = client.get_tree_json(root_tree_hash) + except Exception as e: + logger.debug("skills_sync_client: manifest root read failed: %s", e) + return None + for e in tree.get("entries", []): + if e.get("name") == SYNC_MANIFEST_ENTRY_NAME and e.get("kind") == KIND_BLOB: + try: + _kind, data = client.get_object(e["hash"]) + except Exception as ex: + logger.debug("skills_sync_client: manifest blob fetch failed: %s", ex) + return None + return parse_sync_manifest(data) + return None + + +def _check_version(caps: Dict[str, Any]) -> None: + """Reject an incompatible server major version (sync contract).""" + ver = str(caps.get("hsp_version") or "") # wire field name + major = ver.split(".", 1)[0] + if major != WIRE_VERSION: + raise SyncError( + f"this server speaks sync version {ver!r}, but this Hermes speaks " + f"{WIRE_VERSION} — update Hermes to sync with it" + ) + + +# --------------------------------------------------------------------------- +# Push +# --------------------------------------------------------------------------- + +def push_skills( + client: Optional["SyncClient"] = None, + *, + skill_names: Optional[List[str]] = None, + identity: Optional[Dict[str, Any]] = None, + message: str = "hermes skill sync", +) -> Dict[str, Any]: + """Push opted-in skills to the owner's HEAD (sync contract). + + Uploads all new objects, then CAS-es ``refs/user//HEAD``. On a 409, + fetches the actual head, three-way merges, and retries once (§4.4 / M1-C). + Returns a result dict; never raises for the inert / no-op cases. + """ + if identity is None: + identity = resolve_identity() + owner = identity["owner"] + if client is None: + base = resolve_sync_base_url() + if not base: + return {"ok": False, "reason": "no sync base url configured", "noop": True} + client = SyncClient(base, identity["api_key"]) + + if skill_names is None: + skill_names = list_synced_skill_names() + if not skill_names: + return {"ok": True, "reason": "no skills opted into sync", "noop": True} + + caps = client.capabilities() + _check_version(caps) + max_bytes = int(caps.get("max_object_bytes") or DEFAULT_MAX_OBJECT_BYTES) + + objects, root_hash, _ = snapshot_profile(skill_names, max_object_bytes=max_bytes) + + manifest = read_sync_state() + base_head = manifest.get("head") + + # Idempotency: if the profile-root tree is unchanged since our last push, + # there is nothing to propagate -- skip building an empty commit (contract + # objects are immutable, so an identical tree hash means identical content). + if base_head and manifest.get("root") == root_hash: + return {"ok": True, "head": base_head, "reason": "unchanged", "noop": True} + + device = stable_device_id() + parents = [base_head] if base_head else [] + commit_hash = build_commit( + root_hash, parents, owner=owner, device=device, message=message, objects=objects + ) + + client.put_objects(objects.objects) + ref = user_head_ref(owner) + + try: + client.cas_ref(ref, base_head, commit_hash) + manifest["head"] = commit_hash + manifest["root"] = root_hash + write_sync_state(manifest) + return {"ok": True, "head": commit_hash, "pushed_objects": len(objects)} + except SyncConflict as conflict: + return _resolve_push_conflict( + client, identity, conflict.actual, root_hash, commit_hash, + objects, skill_names, message, base_head, + ) + + +# --------------------------------------------------------------------------- +# Conflict resolution / three-way merge +# +# On a 409 the server hands back the actual head. We fetch it, three-way merge +# per skill against the base we forked from, reusing the origin/user/incoming +# decision semantics of skills_sync.py (_is_tracked_user_modification + +# the decision block at skills_sync.py:619-643): +# +# * base == ours == theirs -> nothing to do +# * ours == base, theirs moved -> take theirs (fast-forward incoming) +# * theirs == base, ours moved -> keep ours (our local edit) +# * both moved, ours == theirs -> converged; take either +# * both moved, differ -> TRUE OVERLAP -> conflict head +# +# Non-overlapping merges (each side changed a DIFFERENT skill) produce a merge +# commit (2 parents) and retry the CAS. A true overlap (both sides changed the +# SAME skill differently) is written to refs/user//conflict/ and +# surfaced for out-of-band resolution. +# --------------------------------------------------------------------------- + +def _resolve_push_conflict( + client: "SyncClient", + identity: Dict[str, Any], + actual_head: str, + our_root: str, + our_commit: str, + objects: "ObjectSet", + skill_names: List[str], + message: str, + base_head: Optional[str], +) -> Dict[str, Any]: + owner = identity["owner"] + device = stable_device_id() + + theirs_root = _root_tree_of_commit(client, actual_head) + base_root = _root_tree_of_commit(client, base_head) if base_head else None + + ours_trees = _skill_trees_of_root(client, our_root) + theirs_trees = _skill_trees_of_root(client, theirs_root) + base_trees = _skill_trees_of_root(client, base_root) if base_root else {} + + merged: Dict[str, str] = {} + overlaps: List[str] = [] + all_paths = set(ours_trees) | set(theirs_trees) | set(base_trees) + for path in all_paths: + o = ours_trees.get(path) + t = theirs_trees.get(path) + b = base_trees.get(path) + decision = _merge_skill(b, o, t) + if decision == "overlap": + overlaps.append(path) + # Keep OURS on the surfaced conflict head; theirs is retained + # server-side under the conflict ref for out-of-band resolution. + if o is not None: + merged[path] = o + elif decision == "ours" and o is not None: + merged[path] = o + elif decision == "theirs" and t is not None: + merged[path] = t + elif decision == "either": + merged[path] = o if o is not None else t # type: ignore[assignment] + # decision == "none": skill deleted on the winning side -> drop + + if overlaps: + # TRUE OVERLAP -> write a conflict head and surface it (personal sync). + n = _next_conflict_index(client, owner) + conflict_ref = user_conflict_ref(owner, n) + try: + client.cas_ref(conflict_ref, None, our_commit) + except SyncConflict: + pass # someone else grabbed this index; the head still exists + return { + "ok": False, + "conflict": True, + "conflict_ref": conflict_ref, + "overlapping_skills": sorted(overlaps), + "actual_head": actual_head, + "message": ( + f"{len(overlaps)} skill(s) changed on both sides; wrote " + f"{conflict_ref}. Resolve out-of-band (hermes sync / NAS UI)." + ), + } + + # Non-overlap -> build a merge commit (parents: base->actual, ours) and + # retry the CAS against the actual head. + merge_objects = ObjectSet() + # Re-add our objects so the merge push is self-contained (idempotent). + for h, (kind, data) in objects.objects.items(): + merge_objects.objects[h] = (kind, data) + merged_root = _assemble_root_from_skill_trees(client, merged, merge_objects) + merge_commit = build_commit( + merged_root, + [actual_head, our_commit], + owner=owner, + device=device, + message=f"merge: {message}", + objects=merge_objects, + ) + client.put_objects(merge_objects.objects) + try: + client.cas_ref(user_head_ref(owner), actual_head, merge_commit) + except SyncConflict as c2: + return { + "ok": False, + "conflict": True, + "message": f"merge CAS lost again (head now {c2.actual}); retry sync.", + "actual_head": c2.actual, + } + manifest = read_sync_state() + manifest["head"] = merge_commit + manifest["root"] = merged_root + write_sync_state(manifest) + return {"ok": True, "head": merge_commit, "merged": True} + + +def _merge_skill(base: Optional[str], ours: Optional[str], theirs: Optional[str]) -> str: + """Three-way decision for one skill's tree hash. + + Returns one of: ``ours``, ``theirs``, ``either``, ``overlap``, ``none``. + Mirrors the origin/user/incoming decision block of skills_sync.py:619-643: + a side "modified" the skill when its hash differs from the common base + (analogous to ``_is_tracked_user_modification(origin, current)``). + """ + if ours == theirs: + return "either" if ours is not None else "none" + ours_changed = ours != base + theirs_changed = theirs != base + if ours_changed and not theirs_changed: + return "ours" + if theirs_changed and not ours_changed: + return "theirs" + # both changed and differ + return "overlap" + + +def _assemble_root_from_skill_trees( + client: "SyncClient", skill_trees: Dict[str, str], objects: "ObjectSet" +) -> str: + """Build a profile-root tree object from ``{posix_rel_path: tree_hash}``. + + Rebuilds the intermediate category trees. The referenced skill trees are + assumed already durable (they came from either side of the merge); only + the new intermediate/root tree objects are added to *objects*. + """ + root: Dict[str, Any] = {} + for path, tree_hash in skill_trees.items(): + parts = PurePosixPath(path).parts + node = root + for part in parts[:-1]: + node = node.setdefault(part, {}) + node[parts[-1]] = {"__tree__": tree_hash} + return _build_root_tree(root, objects) + + +def _next_conflict_index(client: "SyncClient", owner: str) -> int: + """Pick the next free conflict ref index for the owner.""" + try: + refs = client.get_refs(f"refs/user/{owner}/conflict/") + except SyncError: + return 1 + used = [] + for r in refs: + name = r.get("name", "") + tail = name.rsplit("/", 1)[-1] + if tail.isdigit(): + used.append(int(tail)) + return (max(used) + 1) if used else 1 + + +# --------------------------------------------------------------------------- +# Pull +# --------------------------------------------------------------------------- + +def pull_skills( + client: Optional["SyncClient"] = None, + *, + identity: Optional[Dict[str, Any]] = None, +) -> Dict[str, Any]: + """Pull the owner's HEAD and materialize opted-in skills to disk. + + Fetches ``refs/user//HEAD``; if it advanced past our recorded head, + walks the profile-root tree and writes each skill tree into + ~/.hermes/skills/. Only paths the user has opted into (``sync: true``) are + materialized, so a pull never resurrects a skill the user hasn't chosen. + Best-effort; returns a result dict. + """ + if identity is None: + identity = resolve_identity() + owner = identity["owner"] + if client is None: + base = resolve_sync_base_url() + if not base: + return {"ok": False, "reason": "no sync base url configured", "noop": True} + client = SyncClient(base, identity["api_key"]) + + caps = client.capabilities() + _check_version(caps) + + refs = client.get_refs(user_head_ref(owner)) + head = None + for r in refs: + if r.get("name") == user_head_ref(owner): + head = r.get("hash") + break + if not head: + return {"ok": True, "reason": "no remote HEAD yet", "noop": True} + + manifest = read_sync_state() + if head == manifest.get("head"): + return {"ok": True, "reason": "already up to date", "head": head, "noop": True} + + root_tree = _root_tree_of_commit(client, head) + remote_trees = _skill_trees_of_root(client, root_tree) + + # : reconcile local opt-in intent FROM the plane manifest, so a skill the + # user opted in on another device becomes opted in here too (opt-in is + # cross-device content, not a device-local flag). We only ADOPT enables from + # the manifest for skills present in the remote tree; we never silently + # disable a locally-enabled skill on pull (that stays the user's local call + # until their next push reconciles it). + reconciled_from_manifest: List[str] = [] + remote_manifest = read_manifest_of_root(client, root_tree) + if remote_manifest: + try: + from tools.skill_usage import set_sync, is_curation_eligible, is_sync_enabled + + for sname, enabled in remote_manifest.items(): + if not enabled: + continue + if not is_curation_eligible(sname): + continue + if not is_sync_enabled(sname): + set_sync(sname, True) + reconciled_from_manifest.append(sname) + except Exception as e: + logger.debug("skills_sync_client: manifest opt-in reconcile failed: %s", e) + + opted_in = set(_opted_in_rel_paths()) + updated = [] + for path, tree_hash in remote_trees.items(): + # Opt-in gate on pull: only materialize skills the user chose to sync + # (now including any adopted from the plane manifest above). + if opted_in and path not in opted_in: + continue + dest = _skills_dir() / path + materialize_tree(client, tree_hash, dest) + updated.append(path) + + manifest["head"] = head + write_sync_state(manifest) + return { + "ok": True, + "head": head, + "updated": sorted(updated), + "opt_in_adopted": sorted(reconciled_from_manifest), + } + + +def _opted_in_rel_paths() -> List[str]: + """Relative posix paths of skills the user has opted into sync.""" + paths = [] + for name in list_synced_skill_names(): + rel = _skill_rel_path(name) + if rel is not None: + paths.append(rel.as_posix()) + return paths + + +# --------------------------------------------------------------------------- +# Gated public entrypoints (gate-and-swallow) +# +# maybe_pull_skills / maybe_push_skills clone the shape of the curator's +# maybe_run_curator (agent/curator.py:1998): best-effort, never raise, return +# a result dict or None. The access gate is checked first -- sync is inert +# (no push, no pull, no-op) unless the signed-in user is a Nous admin. +# --------------------------------------------------------------------------- + +def maybe_push_skills(*, message: str = "hermes skill sync") -> Optional[Dict[str, Any]]: + """Best-effort push if all gates pass. Returns a result dict or None. + Never raises. Called from the debounced skill_manage push hook.""" + try: + identity = resolve_identity() + if not identity.get("nous_admin"): + return None # access gate: inert unless the user is a Nous admin + if not sync_feature_enabled(): + return None # feature off for this instance (HERMES_SYNC_ENABLED) + if not resolve_sync_base_url(): + return None + if not list_synced_skill_names(): + return None + return push_skills(identity=identity, message=message) + except Exception as e: + logger.debug("skills_sync_client: maybe_push_skills failed: %s", e, exc_info=True) + return None + + +def maybe_pull_skills() -> Optional[Dict[str, Any]]: + """Best-effort pull if all gates pass. Returns a result dict or None. + Never raises. Invoked at the curator tick sites (gateway housekeeping loop + + CLI startup).""" + try: + identity = resolve_identity() + if not identity.get("nous_admin"): + return None # access gate: inert unless the user is a Nous admin + if not sync_feature_enabled(): + return None # feature off for this instance (HERMES_SYNC_ENABLED) + if not resolve_sync_base_url(): + return None + return pull_skills(identity=identity) + except Exception as e: + logger.debug("skills_sync_client: maybe_pull_skills failed: %s", e, exc_info=True) + return None + + +def sync_status() -> Dict[str, Any]: + """Return a status snapshot for ``hermes sync status``. Never raises.""" + status: Dict[str, Any] = { + "nous_admin": False, + "logged_in": False, + "feature_enabled": sync_feature_enabled(), + "default_opt_in": sync_default_opt_in(), + "base_url": resolve_sync_base_url(), + "opted_in_skills": [], + "local_head": None, + "owner": None, + # Org-shared skills. `org_available` is False for an account that + # isn't in a shared organisation — the org workflow does not apply, + # which is different from it being broken or misconfigured. + "org_available": False, + "org_id": None, + "org_role": None, + "org_skills": [], + # Org skills edited locally and not yet shared back. + "org_skills_modified": [], + } + try: + identity = resolve_identity() + status["logged_in"] = True + status["owner"] = identity.get("owner") + status["nous_admin"] = bool(identity.get("nous_admin")) + except SyncInertError: + pass + except Exception as e: + logger.debug("skills_sync_client: sync_status identity failed: %s", e) + try: + status["opted_in_skills"] = list_synced_skill_names() + status["local_head"] = read_sync_state().get("head") + except Exception: + pass + try: + org_identity = resolve_org_identity() + status["org_available"] = True + status["org_id"] = org_identity.get("org_id") + status["org_role"] = org_identity.get("org_role") + status["org_skills"] = list_org_skill_names() + status["org_skills_modified"] = list_locally_modified_org_skills( + status["org_id"] + ) + except SyncInertError: + pass + except Exception as e: + logger.debug("skills_sync_client: sync_status org lookup failed: %s", e) + return status + + +def list_org_skill_names() -> List[str]: + """Skill names present in the local org mirror (empty when none pulled).""" + names: List[str] = [] + try: + from agent.skill_utils import read_active_org_id + + org_id = read_active_org_id(_skills_dir()) + if not org_id: + return names + root = _org_dir() / org_id + if not root.is_dir(): + return names + for skill_md in root.rglob("SKILL.md"): + rel = skill_md.parent.relative_to(root) + if rel.parts: + names.append(str(rel).replace("\\", "/")) + except Exception as e: + logger.debug("skills_sync_client: org skill listing failed: %s", e) + return sorted(names) + + +# --------------------------------------------------------------------------- +# Org-shared skills (sync contract) — org pull + propose. +# +# Org skills live under a DISTINCT local namespace, ~/.hermes/skills/_org/ +# (the design notes: enterprise-managed skills are read-only to the runtime; a +# local edit is a personal fork of record until proposed). The org canonical +# set is `refs/org//HEAD` — the SAME object model as personal sync. +# +# PERSONAL-ORG GATE (the sync contract REFINED, Ben 2026-07-23): a personal org +# has NO org workflow. The discriminator travels in the token: NAS stamps the +# `org_role` claim ONLY for multi-member orgs. No claim ⇒ every org helper +# here is inert (org_sync_available() False; pull/propose raise SyncInertError) +# and the personal personal sync experience is untouched. +# +# `hermes sync propose` is the org sharing surface; proposal is +# intended to become largely automated later (curator/background hooks driving +# the same propose_skill() path). Keep this callable non-interactive. +# --------------------------------------------------------------------------- + +ORG_DIR_NAME = "_org" + + +def resolve_org_identity() -> Dict[str, Any]: + """Resolve identity + org context for org-skill operations. + + Returns ``resolve_identity()``'s dict extended with ``org_id`` and + ``org_role``. Raises :class:`SyncInertError` when the token carries no + ``org_role`` claim (personal org / issuer predates org support) — the + caller should treat org sync as unavailable, NOT as an error. + """ + identity = resolve_identity() + claims = identity.get("claims") or {} + org_id = claims.get("org_id") + org_role = claims.get("org_role") + if not org_id: + raise SyncInertError("no organisation associated with this account") + if not isinstance(org_role, str) or not org_role: + raise SyncInertError( + "this account isn't a member of a shared organisation" + ) + identity["org_id"] = str(org_id) + identity["org_role"] = org_role + return identity + + +def org_sync_available() -> bool: + """True iff this token can see the org-skill surface (multi-member org).""" + try: + resolve_org_identity() + return True + except Exception: + return False + + +def org_head_ref(org_id: str) -> str: + return f"refs/org/{org_id}/HEAD" + + +def _org_dir() -> Path: + """Local mirror root for org skills (read-only by convention ).""" + return _skills_dir() / ORG_DIR_NAME + + +def pull_org_skills( + client: Optional["SyncClient"] = None, + *, + identity: Optional[Dict[str, Any]] = None, +) -> Dict[str, Any]: + """Pull the org canonical set into ``~/.hermes/skills/_org//``. + + Fast-forward only (design.md §2.6: no client merge on the org path): the + mirror is replaced with the org HEAD's content. Local edits under _org/ + are NOT merged — they are overwritten on pull; a member's change of record + is `propose_skill` (the fork lives in their personal skills, not _org/). + Returns {ok, org_id, head, updated} (updated = skill rel-paths written). + """ + identity = identity or resolve_org_identity() + if "org_id" not in identity: + raise SyncInertError("no organisation context available") + org_id = identity["org_id"] + if client is None: + base_url = resolve_sync_base_url() + if not base_url: + raise SyncInertError("no sync base URL configured") + client = SyncClient(base_url, identity["api_key"]) + + caps = client.capabilities() + _check_version(caps) + if "org" not in (caps.get("features") or []): + raise SyncInertError("this server does not support org-shared skills") + + refs = client.get_refs(f"refs/org/{org_id}/") + head = next( + (r["hash"] for r in refs if r.get("name") == org_head_ref(org_id)), None + ) + # TOKEN-GATED resolution marker (agent/skill_utils.read_active_org_id): + # written HERE because this function only runs after resolve_org_identity + # verified the token's org_id + org_role. Discovery scans only the marked + # org's mirror, so a stale mirror from a previous org stops resolving the + # moment a pull runs under a different org — no manual cleanup. + _write_active_org_marker(org_id) + if not head: + return {"ok": True, "org_id": org_id, "head": None, "updated": []} + + head_commit = client.get_commit_json(head) + root_tree = head_commit["tree"] + skill_trees = _skill_trees_of_root(client, root_tree) + + dest_root = _org_dir() / org_id + updated: List[str] = [] + # Skills the user/agent has edited locally and upstream also changed. + # We do NOT overwrite them — the local work wins until the user resolves. + conflicted: List[str] = [] + baseline = _read_org_baseline(org_id) + for rel_path, tree_hash in sorted(skill_trees.items()): + dest = dest_root / PurePosixPath(rel_path) + try: + if dest.exists(): + # Local edits are protected: never clobber work the user or + # agent did in place. Skip the update and report it so they + # can resolve deliberately (propose the local version, or + # discard it and re-pull). + if org_skill_is_locally_modified(rel_path, org_id): + prev = baseline.get(rel_path) or {} + # Upstream also moved on => a real conflict the user must + # resolve. Upstream unchanged => their edit simply stands. + if prev.get("tree") != tree_hash: + conflicted.append(rel_path) + continue + import shutil + + shutil.rmtree(dest) + dest.mkdir(parents=True, exist_ok=True) + materialize_tree(client, tree_hash, dest) + baseline[rel_path] = { + "fingerprint": _skill_dir_fingerprint(dest), + "tree": tree_hash, + } + updated.append(rel_path) + except Exception as e: + logger.warning( + "skills_sync_client: org skill materialize failed for %s: %s", + rel_path, + e, + ) + # Provenance sidecar for the load-time header (skill_view): the HEAD + # commit's author is TOKEN-VERIFIED at push time by the plane + # (author_mismatch guard, the sync plane) — trustworthy to display. + _write_org_provenance( + org_id, + { + "org_id": org_id, + "head": head, + "author_user_id": (head_commit.get("author") or {}).get("owner", ""), + "author_device": (head_commit.get("author") or {}).get("device", ""), + "ts": head_commit.get("ts", ""), + "skills": updated, + }, + ) + _write_org_baseline(org_id, baseline) + if conflicted: + logger.warning( + "skills_sync_client: %d org skill(s) have local edits AND upstream " + "changes; left untouched: %s", + len(conflicted), + ", ".join(conflicted), + ) + return { + "ok": True, + "org_id": org_id, + "head": head, + "updated": updated, + "conflicted": conflicted, + } + + +def _skill_dir_fingerprint(path: Path) -> str: + """Stable content hash of a materialized skill directory. + + Used to tell "the user/agent edited this org skill" from "this is exactly + what upstream shipped". Hashes every file's relative path + bytes, sorted, + so it is independent of filesystem ordering and mtimes. + """ + h = hashlib.sha256() + try: + for f in sorted(p for p in path.rglob("*") if p.is_file()): + h.update(str(f.relative_to(path)).replace("\\", "/").encode("utf-8")) + h.update(b"\0") + h.update(f.read_bytes()) + h.update(b"\0") + except OSError as e: + logger.debug("skills_sync_client: fingerprint failed for %s: %s", path, e) + return "" + return h.hexdigest() + + +def _org_baseline_path(org_id: str) -> Path: + """Sidecar recording the upstream fingerprint of each mirrored skill.""" + from agent.skill_utils import ORG_BASELINE_FILE + + return _org_dir() / org_id / ORG_BASELINE_FILE + + +def _read_org_baseline(org_id: str) -> Dict[str, Any]: + try: + return json.loads(_org_baseline_path(org_id).read_text(encoding="utf-8")) + except Exception: + return {} + + +def _write_org_baseline(org_id: str, baseline: Dict[str, Any]) -> None: + try: + p = _org_baseline_path(org_id) + p.parent.mkdir(parents=True, exist_ok=True) + p.write_text(json.dumps(baseline, indent=2, sort_keys=True), encoding="utf-8") + except Exception as e: + logger.debug("skills_sync_client: baseline write failed: %s", e) + + +def org_skill_is_locally_modified(skill_rel_path: str, org_id: str) -> bool: + """True when the local copy of an org skill differs from what upstream sent.""" + dest = _org_dir() / org_id / PurePosixPath(skill_rel_path) + if not dest.is_dir(): + return False + entry = _read_org_baseline(org_id).get(skill_rel_path) or {} + recorded = entry.get("fingerprint") if isinstance(entry, dict) else entry + if not recorded: + # No baseline recorded (pre-existing mirror) — treat as unmodified so + # we don't cry wolf; the next pull records one. + return False + return _skill_dir_fingerprint(dest) != recorded + + +def list_locally_modified_org_skills(org_id: Optional[str] = None) -> List[str]: + """Org skills with local edits that upstream has not seen.""" + try: + from agent.skill_utils import read_active_org_id + + org_id = org_id or read_active_org_id(_skills_dir()) + if not org_id: + return [] + baseline = _read_org_baseline(org_id) + return sorted( + rel for rel in baseline if org_skill_is_locally_modified(rel, org_id) + ) + except Exception as e: + logger.debug("skills_sync_client: modified-scan failed: %s", e) + return [] + + +def _write_active_org_marker(org_id: str) -> None: + """Record which org's mirror may resolve (best-effort, never raises).""" + try: + from agent.skill_utils import ORG_ACTIVE_MARKER + + root = _org_dir() + root.mkdir(parents=True, exist_ok=True) + (root / ORG_ACTIVE_MARKER).write_text(org_id, encoding="utf-8") + except Exception as e: + logger.debug("skills_sync_client: active-org marker write failed: %s", e) + + +def _write_org_provenance(org_id: str, data: Dict[str, Any]) -> None: + """Persist the org HEAD provenance sidecar (best-effort, never raises).""" + try: + from agent.skill_utils import ORG_PROVENANCE_FILE + + dest = _org_dir() / org_id + dest.mkdir(parents=True, exist_ok=True) + (dest / ORG_PROVENANCE_FILE).write_text( + json.dumps(data, indent=2), encoding="utf-8" + ) + except Exception as e: + logger.debug("skills_sync_client: org provenance write failed: %s", e) + + +def propose_skill( + skill_name: str, + client: Optional["SyncClient"] = None, + *, + identity: Optional[Dict[str, Any]] = None, + message: Optional[str] = None, +) -> Dict[str, Any]: + """Propose a local skill's current content to the org canonical set. + + Snapshots the LOCAL (personal) skill directory as an org-scoped commit + layered on the current org HEAD tree (splice/replace that one skill + subtree), uploads the objects with ``?scope=org``, then CAS-es the org + HEAD (contract §11.5): + + - ADMIN/OWNER token → the server merges directly → ``{ok, merged: True}``. + - MEMBER token → the server converts to a proposal (202) → + ``{ok, proposal_pending: True, proposal_id, ref}``. NEVER presented as + live/merged. + + Non-interactive by design — an automated submitter (curator hook) drives + this exact function later (Ben's automation trajectory). + """ + identity = identity or resolve_org_identity() + org_id = identity["org_id"] + if client is None: + base_url = resolve_sync_base_url() + if not base_url: + raise SyncInertError("no sync base URL configured") + client = SyncClient(base_url, identity["api_key"]) + + caps = client.capabilities() + _check_version(caps) + if "org" not in (caps.get("features") or []): + raise SyncInertError("this server does not support org-shared skills") + max_bytes = int(caps.get("max_object_bytes") or DEFAULT_MAX_OBJECT_BYTES) + + # Locate the local skill directory (personal namespace, NOT _org/). + rel = _skill_rel_path(skill_name) + if rel is None: + raise SyncError(f"skill '{skill_name}' not found under the skills dir") + skill_dir = _skills_dir() / rel + if not (skill_dir / "SKILL.md").exists(): + raise SyncError(f"skill '{skill_name}' has no SKILL.md") + + # Build the proposed skill tree. + objects = ObjectSet() + skill_tree = build_tree(skill_dir, objects, max_object_bytes=max_bytes) + + # Base = current org HEAD (None for the org's first content). The proposed + # root is HEAD's skill-tree map with this one skill spliced in — proposals + # are per-skill deltas, never a wholesale replace of the org set. + refs = client.get_refs(f"refs/org/{org_id}/") + base_head = next( + (r["hash"] for r in refs if r.get("name") == org_head_ref(org_id)), None + ) + if base_head: + base_root = _root_tree_of_commit(client, base_head) + skill_map = _skill_trees_of_root(client, base_root) + else: + skill_map = {} + skill_map[str(rel)] = skill_tree + + root_hash = _assemble_root_from_skill_trees(client, skill_map, objects) + commit_hash = build_commit( + root_hash, + [base_head] if base_head else [], + owner=identity["owner"], + device=stable_device_id(), + message=message or f"propose {skill_name}", + objects=objects, + ) + + client.put_objects(objects.objects, org_scope=True) + result = client.cas_ref(org_head_ref(org_id), base_head, commit_hash) + + if result.get("proposal_pending"): + return { + "ok": True, + "proposal_pending": True, + "proposal_id": result.get("proposal_id"), + "ref": result.get("ref"), + "commit": commit_hash, + "org_id": org_id, + } + return { + "ok": True, + "merged": True, + "head": result.get("hash", commit_hash), + "commit": commit_hash, + "org_id": org_id, + } + + +def maybe_pull_org_skills() -> Optional[Dict[str, Any]]: + """Best-effort org pull if all gates pass. Never raises; None when inert. + + Gates (all must hold): logged in, org_role claim present (multi-member + org), feature enabled, base URL configured. Personal orgs are inert here + by construction — resolve_org_identity raises SyncInertError without the + claim. + + Marker hygiene: when the token VERIFIABLY lacks the org claim (logged in, + personal org / left the org), the active-org marker is cleared so + previously-mirrored org skills stop resolving. When we simply cannot + resolve identity (offline, logged out), the marker is left alone — + offline grace keeps already-pulled org skills working. + """ + try: + identity = resolve_org_identity() + except SyncInertError: + # Distinguish "verifiably personal/left-org" from "can't tell". + try: + base_identity = resolve_identity() + claims = base_identity.get("claims") or {} + if not claims.get("org_role"): + _clear_active_org_marker() + except Exception: + pass # offline/logged out — keep offline grace + return None + except Exception as e: + logger.debug( + "skills_sync_client: maybe_pull_org_skills inert/failed: %s", e + ) + return None + try: + if not sync_feature_enabled(): + return None + if not resolve_sync_base_url(): + return None + return pull_org_skills(identity=identity) + except Exception as e: + logger.debug( + "skills_sync_client: maybe_pull_org_skills inert/failed: %s", e + ) + return None + + +def _clear_active_org_marker() -> None: + """Remove the active-org marker (org skills stop resolving).""" + try: + from agent.skill_utils import ORG_ACTIVE_MARKER + + marker = _org_dir() / ORG_ACTIVE_MARKER + if marker.exists(): + marker.unlink() + logger.info( + "skills_sync_client: cleared active-org marker " + "(token has no org workflow); org skills no longer resolve" + ) + except Exception as e: + logger.debug("skills_sync_client: marker clear failed: %s", e) diff --git a/tools/skills_tool.py b/tools/skills_tool.py index a5613f62c4c87..9943db8160bb5 100644 --- a/tools/skills_tool.py +++ b/tools/skills_tool.py @@ -1561,6 +1561,73 @@ def skill_view( "Could not preprocess skill content for %s", skill_name, exc_info=True ) + # ── M2 org provenance header (load-time) ────────────────────────── + # An org-shared skill announces its provenance IN the returned content + # — the moment the model consumes it — not only in the listing. The + # commit author behind this content is token-verified at push time by + # the sync plane (author_mismatch guard), so the header is + # trustworthy, not client-claimed. Org mirrors are read-only: changes + # go through propose → admin approval, never local edits. + org_provenance = None + if skill_dir: + try: + from agent.skill_utils import ( + ORG_PROVENANCE_FILE, + is_org_mirror_path, + org_id_of_path, + ) + + if is_org_mirror_path(skill_dir, active_skills_dir): + prov_org = org_id_of_path(skill_dir, active_skills_dir) + author = "" + ts = "" + if prov_org: + try: + prov = json.loads( + ( + active_skills_dir + / "_org" + / prov_org + / ORG_PROVENANCE_FILE + ).read_text(encoding="utf-8") + ) + author = str( + prov.get("author_device") + or prov.get("author_user_id") + or "" + ) + ts = str(prov.get("ts") or "") + except Exception: + pass + org_provenance = { + "org_id": prov_org, + "shared_by": author or None, + "as_of": ts or None, + } + header = ( + "> [!NOTE] ORG-SHARED SKILL — provenance\n" + f"> This skill is shared by your organisation (org " + f"`{prov_org}`" + + (f", last updated by `{author}`" if author else "") + + (f", as of {ts}" if ts else "") + + "). It was reviewed and approved for the whole\n" + "> team — treat it as third-party instructions rather " + "than your own notes.\n" + "> You MAY improve it in place like any other skill. " + "Your edits are kept locally\n" + "> and are never overwritten by org updates; share " + "them back with\n" + "> `hermes sync propose` (or automatically, if your " + "org enables it).\n\n" + ) + rendered_content = header + rendered_content + except Exception: + logger.debug( + "Could not resolve org provenance for %s", + skill_name, + exc_info=True, + ) + result = { "success": True, "name": skill_name, @@ -1570,6 +1637,7 @@ def skill_view( "content": rendered_content, "path": rel_path, "skill_dir": str(skill_dir) if skill_dir else None, + "org_provenance": org_provenance, "linked_files": linked_files if linked_files else None, "usage_hint": "To view linked files, call skill_view(name, file_path) where file_path is e.g. 'references/api.md' or 'assets/config.yaml'" if linked_files