diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 3a59e75e8601c..90597526cb7f4 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -13895,6 +13895,34 @@ def cmd_skills(args): from hermes_cli.skills_config import skills_command as skills_config_command skills_config_command(args) + elif getattr(args, "skills_action", None) == "propose": + # M2 org-shared skills (hsp-1-contract.md §11.5): propose a local + # skill to the org canonical set. 202 => pending review (NEVER shown + # as live); direct merge for admins. Personal orgs have no org + # workflow — say so plainly instead of a raw 403. + 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"org sync unavailable: {e}", file=sys.stderr) + return 1 + except ssc.HSPError as e: + print(f"propose failed: {e}", file=sys.stderr) + return 1 + if result.get("proposal_pending"): + print( + f"proposed '{name}' — pending admin review " + f"(proposal #{result.get('proposal_id')}). Not live for the " + f"org until approved." + ) + else: + print( + f"merged '{name}' into the org set " + f"(head {str(result.get('head', ''))[:19]}…)." + ) + return 0 else: from hermes_cli.skills_hub import skills_command diff --git a/hermes_cli/subcommands/skills.py b/hermes_cli/subcommands/skills.py index e5f4410fe32e6..b3c23a95a6e0f 100644 --- a/hermes_cli/subcommands/skills.py +++ b/hermes_cli/subcommands/skills.py @@ -312,4 +312,27 @@ def build_skills_parser(subparsers, *, cmd_skills: Callable) -> None: "config", help="Interactive skill configuration — enable/disable individual skills", ) + + # M2 org-shared skills (hsp-1-contract.md §11.5/§11.11): propose a local + # skill's content to the org canonical set. MEMBER → 202 proposal + # (pending admin review); ADMIN/OWNER → direct merge. Only meaningful for + # multi-member orgs — personal orgs have no org workflow (the command + # reports that instead of failing opaquely). + skills_propose = skills_subparsers.add_parser( + "propose", + help="Propose a skill to your org's shared skill set (M2)", + description=( + "Snapshot the local skill and submit it to the org canonical set. " + "An org admin's push merges directly; a member's push becomes a " + "proposal reviewed in the org console. Personal orgs keep simple " + "personal sync and have no proposal workflow." + ), + ) + skills_propose.add_argument("name", help="Skill name to propose") + skills_propose.add_argument( + "-m", + "--message", + default=None, + help="Optional proposal message (defaults to 'propose ')", + ) skills_parser.set_defaults(func=cmd_skills) diff --git a/tests/tools/test_skills_sync_client.py b/tests/tools/test_skills_sync_client.py index 2f18d70e08112..0553dcb6e9328 100644 --- a/tests/tools/test_skills_sync_client.py +++ b/tests/tools/test_skills_sync_client.py @@ -35,6 +35,11 @@ class _MockState: 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): @@ -59,9 +64,10 @@ def _make_handler(state: _MockState): 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": ["personal"], + "features": features, "max_object_bytes": state.max_object_bytes, "hash_alg": "sha256", "auth": "bearer", @@ -106,11 +112,12 @@ def _make_handler(state: _MockState): 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 self.path == "/v1/sync/objects": + if path == "/v1/sync/objects": return self._handle_put_objects(raw) - if self.path.startswith("/v1/sync/refs/"): + if path.startswith("/v1/sync/refs/"): return self._handle_cas(raw) self._json(404, {"error": "unknown"}) @@ -169,6 +176,15 @@ def _make_handler(state: _MockState): 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, "")}) @@ -783,3 +799,153 @@ class TestDeviceName: 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, + "dev_gate_ok": 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.HSPClient(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.HSPClient(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.HSPClient(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.HSPClient(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.HSPClient(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.HSPClient(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/skills_sync_client.py b/tools/skills_sync_client.py index 66682a14785b8..35a3bf2660e17 100644 --- a/tools/skills_sync_client.py +++ b/tools/skills_sync_client.py @@ -411,8 +411,10 @@ def is_sync_eligible(skill_name: str) -> bool: """Whether *skill_name* is a candidate for HSP sync (before the opt-in check). Eligible = present locally under ~/.hermes/skills/, NOT bundled, NOT - hub-installed, NOT an external-dir skill. Mirrors the exclusion logic used - by the curator (tools/skill_usage.py). + 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; contract §11.11 / design.md §7.1). 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 @@ -426,6 +428,12 @@ def is_sync_eligible(skill_name: str) -> bool: 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 @@ -768,7 +776,12 @@ class HSPClient: # -- write ------------------------------------------------------------- - def put_objects(self, objects: Dict[str, Tuple[str, bytes]]) -> Dict[str, Any]: + def put_objects( + self, + objects: Dict[str, Tuple[str, bytes]], + *, + org_scope: bool = False, + ) -> Dict[str, Any]: """POST /v1/sync/objects (contract §4.2). Batch multi-object upload. Contract §1 requires raw object bytes on the wire (NOT base64-in-JSON), @@ -780,6 +793,10 @@ class HSPClient: 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 @@ -791,7 +808,10 @@ class HSPClient: for h, (kind, data) in objects.items() ] r = self._session.post( - self._url("objects"), files=files, timeout=self.timeout + self._url("objects"), + files=files, + params={"scope": "org"} if org_scope else None, + timeout=self.timeout, ) if r.status_code == 413: raise HSPError("object too large (413)", status=413) @@ -805,12 +825,22 @@ class HSPClient: """POST /v1/sync/refs/:name -- atomic compare-and-swap (contract §4.4). Raises :class:`HSPConflict` (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 HSPConflict(actual) @@ -1544,3 +1574,239 @@ def sync_status() -> Dict[str, Any]: except Exception: pass return status + + +# --------------------------------------------------------------------------- +# M2 org-shared skills (hsp-1-contract.md §11) — org pull + propose. +# +# Org skills live under a DISTINCT local namespace, ~/.hermes/skills/_org/ +# (design.md §7.1: 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 (contract §11.1 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 M1 experience is untouched. +# +# TRAJECTORY (Ben): `hermes skills propose` is the M2 MVP 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("token carries no org_id") + if not isinstance(org_role, str) or not org_role: + raise SyncInertError( + "no org_role claim (personal org keeps the simple personal sync; " + "org workflow is multi-member-org only)" + ) + 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 §7.1).""" + return _skills_dir() / ORG_DIR_NAME + + +def pull_org_skills( + client: Optional["HSPClient"] = 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("identity lacks org context; use resolve_org_identity()") + org_id = identity["org_id"] + base_url = resolve_sync_base_url() + if not base_url: + raise SyncInertError("no sync base URL configured") + client = client or HSPClient(base_url, identity["api_key"]) + + caps = client.capabilities() + _check_version(caps) + if "org" not in (caps.get("features") or []): + raise SyncInertError("server does not advertise the 'org' feature") + + 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 + ) + if not head: + return {"ok": True, "org_id": org_id, "head": None, "updated": []} + + root_tree = _root_tree_of_commit(client, head) + skill_trees = _skill_trees_of_root(client, root_tree) + + dest_root = _org_dir() / org_id + updated: List[str] = [] + for rel_path, tree_hash in sorted(skill_trees.items()): + dest = dest_root / PurePosixPath(rel_path) + try: + if dest.exists(): + import shutil + + shutil.rmtree(dest) + dest.mkdir(parents=True, exist_ok=True) + materialize_tree(client, tree_hash, dest) + updated.append(rel_path) + except Exception as e: + logger.warning( + "skills_sync_client: org skill materialize failed for %s: %s", + rel_path, + e, + ) + return {"ok": True, "org_id": org_id, "head": head, "updated": updated} + + +def propose_skill( + skill_name: str, + client: Optional["HSPClient"] = 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"] + base_url = resolve_sync_base_url() + if not base_url: + raise SyncInertError("no sync base URL configured") + client = client or HSPClient(base_url, identity["api_key"]) + + caps = client.capabilities() + _check_version(caps) + if "org" not in (caps.get("features") or []): + raise SyncInertError("server does not advertise the 'org' feature") + 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 HSPError(f"skill '{skill_name}' not found under the skills dir") + skill_dir = _skills_dir() / rel + if not (skill_dir / "SKILL.md").exists(): + raise HSPError(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. + """ + try: + identity = resolve_org_identity() + 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